508040 - lirasm - Provide global data section support

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, enhancement)

Description

[Paritosh Aggarwal]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.2pre) Gecko/20090729 Ubuntu/9.04 (jaunty) Shiretoko/3.5.2pre
Build Identifier: 

A global storage section for data can be defined at the beginning of a LIR program. This global section can be used to share data amongst fragments, and adds support for storing string data elements directly using usual string syntax.



Reproducible: Always

Comment 1

[Jeff Walden [:Waldo]]

(Aside: Paritosh, I added canconfirm and editbugs to your Bugzilla account as most of the bugs you've filed are in lirasm code whose primary maintainer is you -- so they're almost by definition valid bugs.  :-)  Should save a little time for the rest of us to not have to confirm them...)

Comment 2

[Paritosh Aggarwal]

Attachment: v1

Comment 3

[Paritosh Aggarwal]

Attachment: includes testcase

Comment 4

[Jason Orendorff [:jorendorff]]

I'm sorry I took so long to get around to this review!

It's silly that "x  y" parses as a three-character string, even though there are two spaces. The LirTokenStream should be responsible for parsing strings.  Add a new LirTokenType (STRING), and add parsing code to LirTokenStream::get():

>         } else if (strchr(":,=[]()", c)) {
>             token.type = PUNCT;
>             token.data = c;
>             mLine.erase(0, 1);
>+        } else if (c == '"') {
>+            token.type = STRING;
>+            ???
>+            mLine.erase(0, ???);
>         } else if (c == ';' || c == '\n') {
>             token.type = NEWLINE;
>             token.data.clear();
>             mLine.clear();

Ideally that code would handle JS-like syntax for strings, so if the LIR file contains
  s = .ascii "\n"
then for the token '"\n"', the token.data string would be just one character, (char) 10.

initGlobals should be renamed handleData or handleDataSection.

It seems like this would probably accept many silly input files that it should reject.

.data
  x = z "
.end "s = " "
.end

Please make sure every path through initGlobals either does something useful or calls bad().

Instead of the first line here:
>+            if(!isalpha(mTokens[0][0])) {
>+                ins = mLir->insImm(imm(mTokens[0]));
>+            } else {
write this:
>+            if (mTokens[0].type == NUMBER) {

The memory for globals is allocated using malloc() but never free()d. Go ahead and free it, even though the process is about to exit.

Comment 5

[Luke Wagner [:luke]]

lirasm is gone (from Mozilla, at least).