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

[David Holmes]

Adding back build-dev

On 5/04/2019 2:41 pm, Ioi Lam wrote:
> #define THIS_FILE (__FILE__ + FILE_MACRO_OFFSET)
> 
> This make me a little uneasy, as it could potential point past the end 
> of the string.

The intent of course is that that is impossible:
- _FILE_ is an absolute file path within the repo: /something/repo/file
- FILE_MACRO_OFFSET gets you from / to the top-level repo directory 
leaving "file"

so it has to be in bounds.

> For safety, Is it possible to get some sort of assert to make sure that 
> __FILE__ always has more than FILE_MACRO_OFFSET characters?
> 
> Maybe we can add a static object in macros.hpp with a constructor that 
> puts __FILE__ into a list, and then we can walk the list when the VM 
> starts up? E.g.
> 
>      ...
> 
>      #ifdef ASSERT
>      static ThisFileChecker __this_file_checker(__FILE__);
>      #endif
> 
>      #endif // SHARE_UTILITIES_MACROS_HPP

So basically you're not trusting the compiler and build system to be 
correct here. :)

Would it suffice to put a static assert in a common header, like 
macros.hpp, that verifies the length of _FILE_ or is that not available 
statically?

Cheers,
David

> 
> Thanks
> - Ioi
> 
> 
> On 4/4/19 9:04 PM, David Holmes wrote:
>> Hi Erik,
>>
>> Build and hotspot changes seem okay.
>>
>> Thanks,
>> David
>>
>> On 5/04/2019 8:03 am, Erik Joelsson wrote:
>>>
>>> On 2019-04-04 14:26, Kim Barrett wrote:
>>>>
>>>> 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).
>>>
>>> Right, that makes sense. I was sort of hedging for all possibilities 
>>> here, but as the build logic is currently structured, it will always 
>>> be defined, just sometimes 0.
>>>
>>> New webrev: http://cr.openjdk.java.net/~erikj/8221851/webrev.02/
>>>
>>> /Erik
>>>
>>>> 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.
>>>>
>>>> ------------------------------------------------------------------------------ 
>>>>
>>>>
>>>>
>