[4/6] Disassembly unit test: disassemble one instruction

  On 01/24/2017 03:23 PM, Yao Qi wrote:
> On 17-01-20 00:03:48, Pedro Alves wrote:
>>> +  /* Test gdb_disassembler for a given gdbarch by reading data from a
>>> +     pre-allocated buffer.  If you want to see the disassembled
>>> +     instruction printed to gdb_stdout, set DISASSEMBLER_TEST_VERBOSE
>>> +     to true.  */
>>> +
>>> +  class gdb_disassembler_test : public gdb_disassembler
>>> +  {
>>> +  public:
>>> +
>>> +    const bool DISASSEMBLER_TEST_VERBOSE = false;
>>
>> static.  We give macros long unique names in order to
>> avoid naming conflicts, but if this is no longer a macro,
>> the name could be shortened, to e.g., just:
>>
>>  static const bool verbose = false;
>>
> 
> gdb_disassembler_test is a local class, so it can't have a static data
> member.

The end result is broken then, unfortunately.  I didn't realize it
either until I saw a spurious fail in the gdb.base/unittest.exp test
today.

Running

  gdb -ex "maintenance selftest"

manually I see:

~~~
Type "apropos word" to search for commands related to "word".
warning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default ARC600 settings.

.long 0x256f003fwarning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default ARC600 settings.

.long 0x256f003fwarning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default A6 settings.

.long 0x256f003fwarning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default ARC601 settings.

warning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default ARC700 settings.

brkwarning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default A7 settings.

brkwarning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default ARCv2 settings.

brkwarning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default EM settings.

brkwarning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default HS settings.


brkmov  r0, #0mov       r0, #0mov       r0, #0mov       r0, #0mov       r0, #0mov       r0, #0mov       r0, #0mov       r0, #0mov       r0, #0mov       r0, #0mov       r0, #0mov   r0, #0mov       r0, #0mov       r0, #0mov       r0, #0breakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakbreakM3.L = 0xffff;/* ( -1) M3=0x0xffff(65535) */break 8break 8warning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB.  Attempting to continue with the default cris:common_v10_v32 settings.
~~~

etc.  (this is what shows up in the gdb.log too).

Note the odd bits of instructions printed.

They appear because here:

  class gdb_disassembler_test : public gdb_disassembler
  {
  public:

    const bool verbose = false;

    explicit gdb_disassembler_test (struct gdbarch *gdbarch,
				    const gdb_byte *insn,
				    size_t len)
      : gdb_disassembler (gdbarch,
			  (verbose ? gdb_stdout : &null_stream),
			  gdb_disassembler_test::read_memory),


specifically in this line:

			  (verbose ? gdb_stdout : &null_stream),

"verbose" has not been initialized yet, because the order of initialization
is always base classes first.  I.e. "verbose" is only initialized after
the base constructor is called.  Since the gdb_disassembler_test object
is created on the stack, "verbose" has garbage at that point, and
thus we end up with the stream incorrectly pointing to gdb_stdout.

I was surprised that GCC doesn't warn about this.  I filed a GCC 
bug here:
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79345

I'm not sure whether this was the reason for the unittest.exp
random failure, but I suspect so.  (I failed to save the log,
and also forgot to check whether all selftests had passed.)

The patchlet below fixes the spurious insn printing.  OK?
  

On Thu, Feb 2, 2017 at 4:46 PM, Pedro Alves <palves@redhat.com> wrote:
>
> I'm not sure whether this was the reason for the unittest.exp
> random failure, but I suspect so.  (I failed to save the log,
> and also forgot to check whether all selftests had passed.)
>
> The patchlet below fixes the spurious insn printing.  OK?
>

Yes.