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

[Tim Bell]

On 04/08/19 07:28, Erik Joelsson wrote:

 > I'm now looking for another Hotspot reviewer.
 >
 > http://cr.openjdk.java.net/~erikj/8221851/webrev.04/

I'm not a Hotspot Reviewer, however the build changes look good FWIW.

Tim

> Hello,
>
> On 2019-04-05 15:46, Kim Barrett wrote:
>>> On Apr 5, 2019, at 11:09 AM, Erik Joelsson <erik.joelsson at oracle.com>
>>> wrote:
>>> So to make it clear. This patch now does the following:
>>>
>>> * Removes the setting of -DTHIS_FILE=... from all compilation units
>>> involved in building Hotspot.
>>>
>>> * Introduces THIS_FILE as a macro in Hotspot src which gets just the
>>> filename from the __FILE__ macro.
>>>
>>> * Changes the use of the __FILE__/THIS_FILE macro in exceptions.hpp
>>> to just always use the new THIS_FILE.
>>>
>>> * Introduces the use of -fmacro-file-map, when supported by the
>>> compiler, to rewrite __FILE__ to a path relative to the workspace root.
>>>
>>> The two main improvements from this are that precompiled headers
>>> should start working with GCC again and when building with GCC 8 or
>>> later, we get rid of absolute paths from our binaries.
>>>
>>> /Erik
>> (I've only looked at the src/hotspot changes.)
>>
>> It's not clear there's a reason to put this_file_helper and THIS_FILE
>> anywhere other than in exceptions.hpp. I don't think there's a (known)
>> benefit to using it anywhere else. Using a trimmed __FILE__ doesn't
>> eliminate the "absolute paths embedded in binaries" problem. For that
>> we need -fmacro-file-map or the like (or perhaps give up on absolute
>> paths in the build system, but those are pretty useful in the .cmdfile
>> files at least).
>>
>> Assuming all that, consider instead putting this_file_helper in
>> exceptions.hpp (perhaps with a better name?), don't bother with
>> THIS_FILE, and define THREAD_AND_LOCATION as
>>
>> #define THREAD_AND_LOCATION THREAD, this_file_helper(__FILE__), __LINE__
>>
> Moved to exceptions.hpp, renamed to "basename", and removed the
> THIS_FILE macro. Given that Kim will not be able to review this further,
> I'm now looking for another Hotspot reviewer.
>
> http://cr.openjdk.java.net/~erikj/8221851/webrev.04/
>
> /Erik
>