-
Notifications
You must be signed in to change notification settings - Fork 601
Avoid z/OS function pointer comparison undefined behavior #23399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
I think its better to figure out which Z/OS C compiler flag/linker flag turns on function pointer equality, and throw that cmd line flag into This bug is obvious to understand for any Linux devs. Its basic ELF interposition at work. The integer address you get by typing PL_check[PERL_CK_NULL] is the the tiny jump shim C function that the PLT/GOT scheme requires, so that https://docs.oracle.com/cd/E19683-01/816-1386/6m7qcobkv/index.html#indexterm-315 A possible fix for Z/OS could also be 1 time hacks like this aren't sustainable, and they won't fix any of the For example, this PR didn't catch this line Line 5121 in 9ef5300
How many more of these lines exist in Perl ecosystem? Its pretty well documented Z/OS has plenty of different ABIs inside 1 or more address spaces per process. And Z/OS C stack is a linked list if I read this correctly. https://share.confex.com/share/119/webprogram/Handout/Session11408/Save_area_Conventions.pdf A https://www.ibm.com/docs/en/zos/2.4.0?topic=qualifiers-far-type-qualifier-c-only
maybe this option for ZOS? https://www.ibm.com/docs/en/zos/2.4.0?topic=qualifiers-fdptr-type-qualifier-c-only
So yeah, this is strictly the fault of the Perl 5 interpreter for assuming Instead of adding extra entries to a P5P created array stored in Or properly declare arrays PL_check/PL_ppaddr with a 128 bit pointer type. |
|
http://www.bitsavers.org/pdf/hp/9000_hpux/1991-200x/HP_aC++_Online_Programmers_Guide.pdf Apparently Commercial Unix OSes or perhaps all RISC+CISC CPUs except for ARM and X86, really hate end users trying to do function pointer numeric comparisons across All the special OSes/CPUs have a CC or LD cmd line option to turn on the option to make func ptrs Relocation, JIT, mmap, pointer encryption, COW, ASLR, PIC, ShLibs, --X only, not R-X, weak symbols, lazy symbol resolver, etc, |
Prior to this commit, code in peep.c that checks to see if a PL_check[] function had been overridden would always return true on z/OS. This would turn off optimization for multideref, causing tests to fail that expected it to be on. One solution would be to just not have multideref on that box, and to skip the failing tests. I'd rather not turn off optimizations unless absolutely necessary. The check for being overridden was to simply compare two function pointers for equality. From information relayed to me on an IBM discord z/OS chat channel, function pointer equality comparisons will never compare equal across different translation units. To get a valid comparison, you must use pointers from the same translation unit. I had first looked at the documentation. It required more background knowledge of the jargon used, and the z/OS design than I was willing to spend the time learning. But there may be a way around this, but if so it's complicated. That's when I turned to the chat channel. The PL_check[] table of the function pointers is declared extern, and when peep.c is compiled, it doesn't have access to that table; it gets linked to later. So its different translation units, and so the pointers never will be equal. However within the table itself, all the pointers to function X will have the same address, as it is going to be in the same translation unit. What this commit does is to add three extra elements to PL_check[], initialized with the function pointers that peep.c wants to compare against. Those pointers are unknown to any other code, so will never be changed away from pointing to these function pointers. And the commit changes peep.c to use the appropriate array entry, which will contain a valid value, instead of using the function pointer directly. This solution works for z/OS and all other current implementations. It, unfortunately, isn't a general solution. We would have to do a similar game if there were other function pointers that were compared across translation units. But this appears to be the only current case this happens. As long as the function pointers to be compared are known in the same translation unit, it works. This commit builds upon some ideas from Dave Mitchell and Richard Leach. People on the z/OS chat independently came up with the same general solution.
It's also unlikely that the op_type will be any given value, but I expect the compiler and libc know that. This stresses that it is unlikely some module will customize the handling of these checkers.
7c1bc8f to
efbf2c0
Compare
|
Please read the revised description. I found a z/OS chat channel, and asked about this. There is no compiler option to force things to work as we expect. I also got more clarity about how things work. Pointer comparisons of non-functions work as expected. Function pointer comparison works on pointers compiled in the same translation unit; will never work across translation units. That is the situation here, where PL_check[] is extern. Things like The z/OSers came up with essentially the same solution as this p.r. originally had. But now that I understand it better, I was able to remove some hedging conditionals. |
Prior to this commit, code in peep.c that checks to see if a PL_check[] function had been overridden would always return true on z/OS. This would turn off optimization for multideref, causing tests to fail that expected it to be on.
One solution would be to just not have multideref on that box, and to skip the failing tests. I'd rather not turn off optimizations unless absolutely necessary.
The check for being overridden was to simply compare two function pointers for equality. From information relayed to me on an IBM Discord z/OS chat channel, function pointer equality comparisons will never compare equal across different translation units. To get a valid comparison, you must use pointers from the same translation unit. I had first looked at the documentation. It required more background knowledge of the jargon used, and the z/OS design than I was willing to spend the time learning. But there may be a way around this, but if so it's complicated. That's when I turned to the chat channel.
The PL_check[] table of the function pointers is declared extern, and when peep.c is compiled, it doesn't have access to that table; it gets linked to later. So these are different translation units, and so the pointers never will be equal. However within the table itself, all the pointers to function X will have the same address, as it is going to be in the same translation unit.
What this commit does is to add three extra elements to PL_check[], initialized with the function pointers that peep.c wants to compare against. Those pointers are unknown to any other code, so will never be changed away from pointing to these function pointers.
And the commit changes peep.c to use the appropriate array entry, which will contain a valid value, instead of using the function pointer directly. This solution works for z/OS and all other current implementations.
It, unfortunately, isn't a general solution. We would have to do a similar game if there were other function pointers that were compared across translation units. But this appears to be the only current case this happens. As long as the function pointers to be compared are known in the same translation unit, it works.
This commit builds upon some ideas from Dave Mitchell and Richard Leach. People on the z/OS chat independently came up with the same general solution.
This set of changes does not require a perldelta entry.