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

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

[mgaron at pleora dot com]

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 <Base.h>

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:

00000128 <_ZN7Derived9ModifyIntEi>:

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

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

protected:
        // Base.
        int ModifyInt(int value) override;
 1a4:   30a5fffc        addik   r5, r5, -4
 1a8:   b0000000        imm     0
                        1a8: R_MICROBLAZE_GOT_64        $LTHUNK2
 1ac:   e9940000        lwi     r12, r20, 0
 1b0:   98086000        bra     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.

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

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
IIRC inheritance across shared library borders is problematic.  You might want
to check how vtables resolve.

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

[mgaron at pleora dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

--- Comment #2 from Martin <mgaron at pleora dot com> ---
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:
000001a4 <_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:   30a5fffc        addik   r5, r5, -4
 1a8:   b0000000        imm     0
                        1a8: R_MICROBLAZE_GOT_64        $LTHUNK2
 1ac:   e9940000        lwi     r12, r20, 0
 1b0:   98086000        bra     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:
00000000000000d8 <_ZThn8_N7Derived9ModifyIntEi>:
  d8:   d1002000        sub     x0, x0, #0x8
  dc:   17fffff7        b       b8 <_ZN7Derived9ModifyIntEi> // jumps to
implementation function.

//proper local jump from inside the reference function aarch64, gcc 8.2.0:
00000128 <_ZThn4_N7Derived9ModifyIntEi>:
 128:   e2400004        sub     r0, r0, #4
 12c:   eafffff1        b       f8 <_ZN7Derived9ModifyIntEi> // again


//proper local jump from inside the reference function x86_64, gcc 5.4.0:
00000000000000c8 <_ZThn8_N7Derived9ModifyIntEi>:
  c8:   48 83 ef 08             sub    $0x8,%rdi
  cc:   eb e6                   jmp    b4 <_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] Order of multiple inheritance can lead to illegal code jump

[mgaron at pleora dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

--- Comment #3 from Martin <mgaron at pleora dot com> ---
Created attachment 50898
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50898&action=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

[mgaron at pleora dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

--- Comment #4 from Martin <mgaron at pleora dot com> ---
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 target/100754] Order of multiple inheritance can lead to illegal code jump

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100754

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The bug/issue is most likely inside microblaze_asm_output_mi_thunk .