microsoft / WinObjC

Objective-C for Windows
MIT License
6.24k stars 811 forks source link

libobjc2 can calculate incorrect ivar offsets #1208

Open edvv opened 7 years ago

edvv commented 7 years ago

Dear WinObjC Engineers,

I have attached a small example Xcode project (rterror.zip) which produces a libobjc2 runtime parsing crash on a computer here. Running vsimporter.exe on it and running the resulting solution demonstrates the crash. I have also attached the console output (rtconsole.txt) showing the Warning and Error lines emitted from libobjc2 parsing code.

See also: https://github.com/Microsoft/WinObjC/issues/1203 and https://github.com/Microsoft/WinObjC/issues/1141

Sincerely,

Ed

rtconsole.txt rterror.zip

rajsesh commented 7 years ago

We have seen this before, @DHowett-MSFT might have some ideas.

DHowett-MSFT commented 7 years ago

Alright, here's the deal:

Classes with negative ivars are totally legal:

Take, for example, this inheritance hierarchy:

@interface X {
    uint8_t ivar1;
    /* padding! */
}
@end

@interface Y: X {
    uint8_t ivar2;
    uint8_t ivar3;
}
@end

Since X has a lot of known padding, Y's ivars can consume it.

   0        1                          4
X: +--------+--------+--------+--------+
   | ivar1  | -PAD-  | -PAD-  | -PAD-  |
   +--------+--------+--------+--------+
           -3       -2       -1
Y:          +--------+--------+
            | ivar2  | ivar3  |
            +--------+--------+

On occasion, however, our compiler will emit invalid negative offsets which consume nonexistent padding:

@interface X {
    uint8_t ivar1;
    /* padding! */
}
@property (assign) void* pointer;
@end

@interface Y: X {
    uint8_t ivar2;
}
@end
   0           4                          8
X: +------<pad>+--------+--------+--------+
   | ivar1     | pointer                  |
   +------<pad>+--------+--------+--------+
Y:                     -3       -2
                        +--------+
                        | ivar2  |
                        +--------+

To work around this, we added a negative ivar slide fixup in libobjc2. Unfortunately, we didn't account for the case where subclass Y has two small ivars without padding between them. Those would be laid out at -3 and -2, respectively.

Projected fix:

  1. Fix ivar sliding for multiple padding-packed ivars in libobjc2. This will waste a few bytes per instance per misaligned class.
  2. Figure out why the compiler is not factoring synthesized properties into layout derivation.

I'll leave this bug open to track 1 and open a new issue for 2.

edvv commented 7 years ago

@DHowett-MSFT Thank you for looking into this and the future fix. Your analysis is consistent with the workaround here, which is to manually add 32 bit padding in the ivar list to prevent the crashing. Sincerely, Ed

DHowett-MSFT commented 7 years ago

This is gnustep/libobjc2#25, gnustep/libobjc2#27. It's fixed upstream and requires us to merge.