[14] RFR(S): 8234583: PrintAssemblyOptions isn't passed to hsdis library

[Schmidt, Lutz]

Hi Vladimir, 

I'm happy to elaborate in more detail about the issue and the fix.

For each decode_env instance which is constructed, process_options() is called. It collects the disassembly options from various sources (Disassembler::pd_cpu_opts() and PrintAssemblyOptions), storing them in the private member "char _option_buf[512]".

Further processing derives static flag settings from these options. Being static, these flags need to be set only once, not every time a decode_env is constructed. 

But that's just one part of the story. It was not taken into account that _option_buf is passed to and analyzed by hsdis-<platform>.so as well. That requires _option_buf to be filled every time a decode_env is constructed. 

Moving
  if (_optionsParsed) return;
after the collect_options() calls heals this deficiency.

I added the guards you question as additional "safety net". After looking at the code again I must admit the guards are not necessary. _option_buf can never be NULL and every invocation of process_options() is directly preceded by a memset(_option_buf, 0, sizeof(_option_buf)). I can remove the guards if you like. 

Please let me know if there are any more questions to be answered. 

Thanks,
Lutz


On 25.11.19, 17:05, "Vladimir Ivanov" <vladimir.x.ivanov at oracle.com> wrote:

    Lutz,
    
    Can you elaborate, please, how the patch fixes the problem?
    
    Why did you decide to add the following guards?
    
    +  if ((options() == NULL) || (strlen(options()) == 0)) {
    
    Best regards,
    Vladimir Ivanov
    
    On 25.11.2019 17:06, Schmidt, Lutz wrote:
    > Dear all,
    > 
    > may I please request reviews for this small change, fixing a regression in the disassembler. Parameters to the hsdis-<platform> library were not passed on.
    > 
    > The change was verified to fix the issue by the reporter (Jean-Philippe Bempel, on CC:). jdk/submit tests pending...
    > 
    > Bug:    https://bugs.openjdk.java.net/browse/JDK-8234583
    > Webrev: https://cr.openjdk.java.net/~lucy/webrevs/8234583.00/
    > 
    > Thank you,
    > Lutz
    >