Bug 1566623
| Summary: | glibc: Old-style function definitions without prototype in libio/strops.c | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Florian Weimer <fweimer> |
| Component: | glibc | Assignee: | Florian Weimer <fweimer> |
| Status: | CLOSED ERRATA | QA Contact: | Sergey Kolosov <skolosov> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 7.6 | CC: | ashankar, codonell, dj, fweimer, mcermak, mnewsome, pfrankli, skolosov |
| Target Milestone: | rc | Keywords: | Patch |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | glibc-2.17-225.el7 | Doc Type: | No Doc Update |
| Doc Text: |
undefined
|
Story Points: | --- |
| Clone Of: | Environment: | ||
| Last Closed: | 2018-10-30 09:38:06 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 1505492 | ||
The problem is that the previous function protoype in libioP.h provided a prototype which under GNU C extensions would override the prototype in libio/strops.c definition, this meant the ABI was consistent. The ABI change here is that in the old-style definition the `int size` is promoted from a sub-word 32-bit type on x86_64 to a full word 64-bit long type, and that may allow the compiler to optimization in certain ways it had not before, perhaps expecting the caller to now clear the top half of the 64-bit register. It's not entirely clear what the ABI impact might be, but it is a change from what we had before and should be fixed. As of *today* the rhel-7.4 and rhel-7.5 implementations are identical, but it might change if the compiler changes: In rhel-7.4 (before the change): 000000000007a040 <_IO_str_init_readonly>: _IO_str_init_readonly(): 7a040: 85 d2 test %edx,%edx 7a042: b8 ff ff ff ff mov $0xffffffff,%eax 7a047: 53 push %rbx 7a048: 0f 49 c2 cmovns %edx,%eax 7a04b: 48 89 fb mov %rdi,%rbx 7a04e: 31 c9 xor %ecx,%ecx 7a050: 48 63 d0 movslq %eax,%rdx 7a053: e8 28 ff ff ff callq 79f80 <_IO_str_init_static_internal> 7a058: 83 0b 08 orl $0x8,(%rbx) 7a05b: 5b pop %rbx 7a05c: c3 retq 7a05d: 0f 1f 00 nopl (%rax) In rhel-7.5: 000000000007ea70 <_IO_str_init_readonly>: _IO_str_init_readonly(): 7ea70: 85 d2 test %edx,%edx 7ea72: b8 ff ff ff ff mov $0xffffffff,%eax 7ea77: 53 push %rbx 7ea78: 0f 49 c2 cmovns %edx,%eax 7ea7b: 48 89 fb mov %rdi,%rbx 7ea7e: 31 c9 xor %ecx,%ecx 7ea80: 48 63 d0 movslq %eax,%rdx 7ea83: e8 28 ff ff ff callq 7e9b0 <_IO_str_init_static_internal> 7ea88: 83 0b 08 orl $0x8,(%rbx) 7ea8b: 5b pop %rbx 7ea8c: c3 retq 7ea8d: 0f 1f 00 nopl (%rax) In rhel-7.4 (before the change): 000000000007a020 <_IO_str_init_static>: _IO_str_init_static(): 7a020: 85 d2 test %edx,%edx 7a022: b8 ff ff ff ff mov $0xffffffff,%eax 7a027: 0f 49 c2 cmovns %edx,%eax 7a02a: 48 63 d0 movslq %eax,%rdx 7a02d: e9 4e ff ff ff jmpq 79f80 <_IO_str_init_static_internal> 7a032: 0f 1f 40 00 nopl 0x0(%rax) 7a036: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 7a03d: 00 00 00 In rhel-7.5: 000000000007ea50 <_IO_str_init_static>: _IO_str_init_static(): 7ea50: 85 d2 test %edx,%edx 7ea52: b8 ff ff ff ff mov $0xffffffff,%eax 7ea57: 0f 49 c2 cmovns %edx,%eax 7ea5a: 48 63 d0 movslq %eax,%rdx 7ea5d: e9 4e ff ff ff jmpq 7e9b0 <_IO_str_init_static_internal> 7ea62: 0f 1f 40 00 nopl 0x0(%rax) 7ea66: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 7ea6d: 00 00 00 In all the functions both before and after the function implementation in assembly expects and manipulates the size in the 32-bit register. This is enough evidence for me *not* to issue a z-stream to fix this, in that the compiler has not done any promotion or adjustment. The key important part is the 'movslq' sign extension from the 32-bit eax into rdx as the internal _IO_size_t is a 64-bit type, and so with this exention the top part of the incoming edx size is not relevant. Therefore while it is an ABI break it appears to be, at present, a benign one, but we *must* fix it :-) I diffed the other architectures as well (7.5 baseline and fixed builds), and I do not see changes in the assembly either. So there is no ABI changes on the relevant architectures in practice. Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHSA-2018:3092 |
glibc-rh1398413.patch removed the function prototypes from libio/libioP.h: -extern void _IO_str_init_static (struct _IO_strfile_ *, char *, int, char *) - __THROW; -extern void _IO_str_init_readonly (struct _IO_strfile_ *, const char *, int) - __THROW; Upstream, this did not have any effect because the function definitions were prototypes, but downstream, we have: 70 void 71 _IO_str_init_static (sf, ptr, size, pstart) 72 _IO_strfile *sf; 73 char *ptr; 74 int size; 75 char *pstart; 76 { 77 return _IO_str_init_static_internal (sf, ptr, size < 0 ? -1 : size, pstart); 78 } 79 80 void 81 _IO_str_init_readonly (sf, ptr, size) 82 _IO_strfile *sf; 83 const char *ptr; 84 int size; 85 { 86 _IO_str_init_static_internal (sf, (char *) ptr, size < 0 ? -1 : size, NULL); 87 sf->_sbf._f._IO_file_flags |= _IO_NO_WRITES; 88 } This results in: strops.c:71:1: warning: function declaration isn't a prototype [-Wstrict-prototypes] strops.c:81:1: warning: function declaration isn't a prototype [-Wstrict-prototypes] It is a potential ABI change, depending on the architecture. The fix is to convert the functions away from the old-style function definition.