RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[Kim Barrett]

> On Apr 4, 2019, at 9:36 AM, Erik Joelsson <erik.joelsson at oracle.com> wrote:
> 
> Hello Kim,
> 
> On 2019-04-03 16:34, Kim Barrett wrote:
>>> On Apr 3, 2019, at 9:51 AM, Erik Joelsson <erik.joelsson at oracle.com> wrote:Additionally, I would like to replace all (or at least most) instances of __FILE__ with my new THIS_FILE, and I suspect some other places would be more performance sensitive. Do you see any problem with doing that substitution?
>> Given that our build system is mostly unhappy with basename collisions
>> (other than open/custom overrides), I don't think there's much of a
>> usage problem with using a basename-like approach everywhere either.
> 
> It's also allowed, though perhaps not used, to override less specific files with more specific, as in OS specific file foo.cpp would override shared foo.cpp.

I didn’t know that.  I don’t think that feature is being used, though could be mistaken.

> I won't update the other references in this patch. I will see if anyone else thinks it's a good enough idea to give it a go. That kind of change would not affect the build, nor the full paths in binaries, just (hopefully) improve readability of error/log messages.

OK.

>> While researching what various platforms do in this area, I ran across
>> a suggestion that debug builds should maybe use full paths and release
>> builds use shortened paths.  That seems like an interesting idea, though
>> we still want short paths in the exceptions.hpp case.
> If you really think this is worth it, I could implement it, but I would rather keep it the same if possible.

Not sure.  The point would be to provide full paths in builds that developers are
using, but not expose full paths in shipped binaries.  But I don’t have a strong opinion.

>>>> This doesn't eliminate the full pathname string embedded in the
>>>> executable, but I don't think the proposed FILE_MACRO_OFFSET will do
>>>> that either.  Those get fixed by -fmacro-prefix-map=.
>>> Correct. While solving this universally would be nice, it wasn't the goal of this bug. The main goal was to get precompiled headers to work with GCC again.
>> I agree that either approach does that.  I looked at the webrev and
>> it's a bunch of build system changes that I don't feel qualified to
>> review, so I suggested an alternative that I understand.  We each have
>> our preferred tools.  I don't object to the build-system-based
>> approach, just can't give an actual thumbs-up.
> 
> Right, and I appreciate the input! But could you at least review the hotspot part, so I can get someone else to review the build part?

OK, I can do that.

------------------------------------------------------------------------------
src/hotspot/share/utilities/macros.hpp
 645 #if FILE_MACRO_OFFSET
 646 #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
 647 #else
 648 #define THIS_FILE __FILE__
 649 #endif

Is the "#if FILE_MACRO_OFFSET" an intentional test for 0, or is this
an implicit test for "defined"?

If the former, e.g. we're assuming it will always be defined but might
have a 0 value, then I'd skip it and just unconditionally define
THIS_FILE as (__FILE__ + FILE_MACRO_OFFSET).

If the latter, some compilers will (with some warning levels or
options, such as gcc -Wundef) complain about the (specified by the
standard) implicit conversion to 0 for an unrecognized identifier in
an #if expression, and an #ifdef should be used to protect against
that.

------------------------------------------------------------------------------