[Bug c++/100754] Order of multiple inheritance can lead to illegal code jump

2021-05-31 Thread mgaron at pleora dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

--- Comment #4 from Martin  ---
Some additional reading hinted me how the intermediate representation can be
help or is enough in itself to reproduce the bug.

I have attached the intermediate representation for the Derived object file, by
adding -save-temps to the g++ compile line.

[Bug c++/100754] Order of multiple inheritance can lead to illegal code jump

2021-05-31 Thread mgaron at pleora dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

--- Comment #3 from Martin  ---
Created attachment 50898
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50898=edit
Intermediate representation of "Derived" class.

Achieved by adding -save-temps to the g++ compile line.

[Bug c++/100754] Order of multiple inheritance can lead to illegal code jump

2021-05-26 Thread mgaron at pleora dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

--- Comment #2 from Martin  ---
Hi Richard,

Thanks for the quick response. I had to do a bit of research to try to properly
express my suspicions.

1st the function from the dispatch table is properly called. Not problems
there:
01a4 <_ZThn4_N7Derived9ModifyIntEi>:

It's the body of this reference function that seems wrong. It should do a local
relative jump to the implementation address, but instead performs a
global relocation, making the implementation function's address change based on
the current shared object it's being called from.

// Base.
int ModifyInt(int value) override;
 1a4:   30a5fffcaddik   r5, r5, -4
 1a8:   b000imm 0
1a8: R_MICROBLAZE_GOT_64$LTHUNK2
 1ac:   e994lwi r12, r20, 0
 1b0:   98086000bra r12


We x-compile and natively compile similar code on a few platforms and only the
microblaze one generates such code. For instance, the sample program attached
in the bug entry generates the following code on different compilers:

//proper local jump from inside the reference function aarch64, gcc8.2.0:
00d8 <_ZThn8_N7Derived9ModifyIntEi>:
  d8:   d1002000sub x0, x0, #0x8
  dc:   17f7b   b8 <_ZN7Derived9ModifyIntEi> // jumps to
implementation function.

//proper local jump from inside the reference function aarch64, gcc 8.2.0:
0128 <_ZThn4_N7Derived9ModifyIntEi>:
 128:   e244sub r0, r0, #4
 12c:   eaf1b   f8 <_ZN7Derived9ModifyIntEi> // again


//proper local jump from inside the reference function x86_64, gcc 5.4.0:
00c8 <_ZThn8_N7Derived9ModifyIntEi>:
  c8:   48 83 ef 08 sub$0x8,%rdi
  cc:   eb e6   jmpb4 <_ZN7Derived9ModifyIntEi> // and
again...


So these last 3 compilers properly generated the local relocation, where the
microblaze code generates global relocation for that thunk. Wouldn't that point
to the microblaze backend?

Also, it is highly suspicious that inverting the order of the parent classes
changes the generated code. And that would point at the gcc front end?

[Bug c++/100754] New: Order of multiple inheritance can lead to illegal code jump

2021-05-25 Thread mgaron at pleora dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

Bug ID: 100754
   Summary: Order of multiple inheritance can lead to illegal code
jump
   Product: gcc
   Version: 9.2.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: mgaron at pleora dot com
  Target Milestone: ---

First time filing a bug, sorry for anything wrong I might be doing.

Experienced with microblaze gcc v8.2.0, v9.2.0, on our existing code base. I
made a simpler example program for sake of debugging.


Simple Base class, with pure virtual function. Compiled in its own shared
library:

class Base {
public:
Base(int value);
virtual ~Base() {}

int GetValue(void);

protected:
virtual int ModInt(int value) = 0;

private:
int value;
};


When used to create a derived class with multiple inheritance, the code
generated seems to be wrong when the base class is specified after some other
interface:

This class too is compiled in its own shared library.

#include 

class ILocalInterface {
public:
virtual ~ILocalInterface() {}
virtual void PrintMsg(void) = 0;
};


class Derived
: public ILocalInterface, public Base {// This leads to a program
crash!
//: public Base, public ILocalInterface { // This works fine...

public:
Derived(int value);
~Derived() {}

// ILocalInterface
void PrintMsg(void) override;

protected:
// Base.
int ModifyInt(int value) override;
};

Used as-is in some test application, any call to ModInt() will cause an illegal
instruction or segfault error.

I created a dump of the .o file and got this:

0128 <_ZN7Derived9ModifyIntEi>:

int Derived::ModifyInt(int value) {
 128:   3021ffdcaddik   r1, r1, -36
...

01a4 <_ZThn4_N7Derived9ModifyIntEi>:
// ILocalInterface
void PrintMsg(void) override;

protected:
// Base.
int ModifyInt(int value) override;
 1a4:   30a5fffcaddik   r5, r5, -4
 1a8:   b000imm 0
1a8: R_MICROBLAZE_GOT_64$LTHUNK2
 1ac:   e994lwi r12, r20, 0
 1b0:   98086000bra r12


_ZThn4_N7Derived9ModifyIntEi symbol does get call appropriately, but it
computes the address to jump to (_ZN7Derived9ModifyIntEi) based on an offset in
the GOT. r20 is the register that holds the GOT for microblaze. When called,
r20 has the value of the GOT of the Base.so library, while it should have the
value of the Derived.so library.

All other compilers I tried (arm64, arm32, x86_64) issue a simple local jump
relative to the PC (2 instructions). Microblaze does support such jump. So in
effect, this might have better to do with the Microblaze backend than the C++
frontend. Let me know if I should modify the affected component.

Also, swapping the order of the interfaces in the declaration of the Derived
class, no such assembly as above is created. Neither is it the case with simple
inheritance.

To me it looks highly suspicious that microblaze generates different code based
on the order of inheritance. Also that the faulty code looks nothing like the
other compiler backends.

Will be happy to provide any assistance, further tests, patch trials, etc.