Need reviewers: more predictable binaries

Kelly O'Hair kelly.ohair at oracle.com
Fri Sep 7 00:39:01 UTC 2012


On Sep 6, 2012, at 1:57 PM, John Coomes wrote:

> Kelly O'Hair (kelly.ohair at oracle.com) wrote:
>> Yes, but I feel like I need to get some kind of approval from a higher level before I try and declare that
>> use of THIS_FILE is a 'jdk convention', and that will trigger long debates :^(
>> 
>> I also need to deal with hotspot, which is a bit trickier because of the macros being expanded in include
>> files (inline method bodies inside *.hpp files).
> 
> There is also the fact that the basenames are not unique.  Maybe it
> won't be an issue, but someone should really check.

They aren't necessarily. But even if they were relative paths, you can't guarantee they would be
unique then. The __FILE__ value is basically not well defined, anyone depending on it is probably
in trouble.

> 
> <wear_kevlar_suit>
> Have you looked at normalizing to the path relative to the repo root?
> </wear_kevlar_suit>

If I could come up with a simple formula that did not involve a repeated exec of sed or a shell for
every compilation, I could change THIS_FILE to a 'relative to the repo root' path.
Just haven't come up with a good solution yet. But that could be an improvement later, e.g.
  ROOT_DIR:=$(shell hg root)  # Or set it in top Makefile somehow?
   -DTHIS_FILE='"$(subst $(ROOT_DIR)/,./,$(@D))$(@F)"'
might work, but it's a bit more complicated than that.

-kto

> 
> -John
> 
>> On Sep 5, 2012, at 11:59 PM, Fredrik Öhrström wrote:
>> 
>>> Looks good. Perhaps we can even remove the "#ifndef THIS_FILE" test in the source files? At some time in the future….
>>> 
>>> //Fredrik
>>> 
>>> 6 sep 2012 kl. 06:08 skrev Kelly O'Hair:
>>> 
>>>> 
>>>> Need a reviewer for this change.
>>>> 
>>>> http://cr.openjdk.java.net/~ohair/openjdk8/jdk8-this-file/webrev/
>>>> 
>>>> It does change source, but it's effectively a build change.
>>>> 
>>>> The goal here is to try and create more predictable binary files and remove the possibility that
>>>> a full source path (via __FILE__) gets baked into the shared libraries or executables shipped.
>>>> 
>>>> So two changes:
>>>> * sort the .o files during links so they are always provided to the linker in the same order.
>>>> * replace use of __FILE__ to the macro THIS_FILE with just the basename of the source file being compiled
>>>> 
>>>> The __FILE__ issue is that it has an implementation defined string literal value, depending on the compiler
>>>> and the filename supplied on the compile line. By changing to the new THIS_FILE macro, the object files
>>>> will always have a consistent string literal in them, making it easier to compare binaries between builds.
>>>> Something we have never been able to do in the past. Granted it's just the basename for the file, but should be enough.
>>>> The tricky part is that __FILE__ only gets evaluated when it is used. In normal C code, it will appear in
>>>> macros but it only will get evaluated in the source file being compiled. It is rare that macros using
>>>> __FILE__  will get expanded in include files and I detected this not happening in the jdk source at all.
>>>> 
>>>> -kto
>>> 
>> 




More information about the build-dev mailing list