941094 - Further sixgill fixes for building browser

Status: ASSIGNED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

I have a few more fixes in the version of sixgill I am using to analyze the browser. I'll attach them here.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Fix 32-bit compilation failure with %lu format used for 64-bit values

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Handle more combinations of preprocessor + dependency generation

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Simplify chain-exec

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Add option to prepend mangled name to functions

This one probably ought to have an actual review. This adds an option to prefix function names with the mangled name, to make the callgraph computation accurate.

Comment 5

[Brian Hackett [Laid off!]]

Comment on attachment 8349508 [details] [diff] [review]
Add option to prepend mangled name to functions

Review of attachment 8349508 [details] [diff] [review]:
-----------------------------------------------------------------

::: gcc/variable.c
@@ +39,5 @@
>    return false;
>  }
>  
> +static const char*
> +GlobalName(tree decl)

Nit: this declaration should be on one line

::: gcc/xgill.h
@@ +70,5 @@
>  // set to zero to disable generation of record types.
>  extern int xil_generate_record_types;
>  
> +// set to one to prefix function names with their mangled names
> +extern bool xil_prefix_with_mangled;

Can you just make usage of the name mangling always-on and remove this option?  If gcc's mangled names are always available then we should always be using them --- all the stuff to compute our own decorated names for functions was done to workaround a lack of mangled names.

Comment 6

[Steve Fink [:sfink] [:s:]]

(In reply to Brian Hackett (:bhackett) from comment #5)
> 
> Can you just make usage of the name mangling always-on and remove this
> option?  If gcc's mangled names are always available then we should always
> be using them --- all the stuff to compute our own decorated names for
> functions was done to workaround a lack of mangled names.

They appear to be. I don't know if they always have been.

I added the flag mainly because I wasn't sure what else sixgill was being used for, so it was a simple way to maintain backwards-compatibility. You're saying it's ok to always output the mangled name?

Currently, if you set the mangle option, you'll get both the mangled and computed names. If you don't need to preserve the current (generated name only) behavior, maybe it's worth having an option to either generate only the mangled name, or the pair of names?

I was keeping around both the mangled names and the computed names because a number of things still use the "readable" names -- annotations, constructor/destructor parsing and matching, and the final output. Annotations do some rudimentary parsing that is much easier on the unmangled names.

However, that's not a very good reason to maintain a large chunk of code in sixgill and make its output more complex -- sixgill could produce only mangled names, and I could collect them and pipe them all through |demangle| to generate a lookup table. We'd probably lose some typedeffing as compared to the current names, but that's a good thing half the time. Would you prefer that? It'd certainly simplify things to have a 1-1 correspondence between mangled and readable names like we used to pretend to have.

Would you prefer that?

Comment 7

[Brian Hackett [Laid off!]]

Ideally sixgill would only use the fully mangled names for its output, with the rooting analysis demangling names as necessary --- it looks like there is some API (http://gcc.gnu.org/onlinedocs/libstdc++/manual/ext_demangling.html) for this so you could do this on demand without needing to pipe / store information ahead of time.  It's not that big a deal though but in sixgill itself I would like to minimize the number of moving parts and just have one format for the output decorated names.  There aren't any other uses of sixgill which this change would impact.