| Message ID | e1ebc6fa-b1a2-758d-ac38-22e8205ebe12@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Revert Intel CET changes to __jmp_buf_tag (Bug 22743) | expand |
On Wed, Jan 24, 2018 at 8:53 PM, Carlos O'Donell <carlos@redhat.com> wrote: > On 01/24/2018 05:48 PM, Dmitry V. Levin wrote: >> I'm afraid by Monday it will be too late for 2.27 as we will get very >> little testing before the release. > Before reverting: > > [carlos@athas tst-cleanup1]$ /home/carlos/build/glibc/elf/ld.so --library-path /home/carlos/build/glibc:/home/carlos/build/glibc/elf:/home/carlos/build/glibc/dlfcn:/home/carlos/build/glibc/nptl ./tst-cleanup1 > ch (3) > ch (2) > ch (1) > Didn't expect signal from child: got `Segmentation fault' > > After reverting: > > [carlos@athas tst-cleanup1]$ /home/carlos/build/glibc-reverted/elf/ld.so --library-path /home/carlos/build/glibc-reverted:/home/carlos/build/glibc-reverted/elf:/home/carlos/build/glibc-reverted/dlfcn:/home/carlos/build/glibc-reverted/nptl ./tst-cleanup1 > ch (3) > ch (2) > ch (1) > > ~~~ Commit message ~~~ > In commit cba595c350e52194e10c0006732e1991e3d0803b and commit > f81ddabffd76ac9dd600b02adbf3e1dac4bb10ec, ABI compatibility with > applications was broken by increasing the size of the on-stack > allocated __pthread_unwind_buf_t beyond the oringal size. > Applications only have the origianl space available for > __pthread_unwind_register, and __pthread_unwind_next to use, > any increase in the size of __pthread_unwind_buf_t causes these > functions to write beyond the original structure into other > on-stack variables leading to segmentation faults in common > applications like vlc. The only workaround is to version those > functions which operate on the old sized objects, but this must > happen in glibc 2.28. > > Thank you to Andrew Senkevich, H.J. Lu, and Aurelien Jarno, for > submitting reports and tracking the issue down. > > The commit reverts the above mentioned commits and testing on > x86_64 shows that the ABI compatibility is restored. A tst-cleanup1 > regression test linked with an older glibc now passes when run > with the newly built glibc. Previously a tst-cleanup1 linked with > an older glibc would segfault when run with an affected glibc build. > > Tested on x86_64 with no regressions. > > Signed-off-by: Carlos O'Donell <carlos@redhat.com> > ~~~ > > Patch attached. > > OK to commit? > > This fixes the last blocker for glibc 2.27. Please don't revert my patch. Please try this patch: https://sourceware.org/git/?p=glibc.git;a=commit;h=4b7fc470a6740808b41502d7431f91805e272d26 instead. I will clean it up and submit it tomorrow. Thanks.
On 01/25/2018 06:33 AM, H.J. Lu wrote: > Please don't revert my patch. Please try this patch: > > https://sourceware.org/git/?p=glibc.git;a=commit;h=4b7fc470a6740808b41502d7431f91805e272d26 > > instead. I will clean it up and submit it tomorrow. I don't see how adding a symbol version to pthread_create helps to solve the general case. Callers of pthread_register_cancel and pthread_create are often compiled at different times. Not everyone does a mass rebuild each time they switch to a new glibc version. I still think you are over-engineering this. The pad array has still an unused member (the last one). Just change sigsetjmp to store the shadow pointer in that location, then the old and new setjmp will work with the current stack layout. As far as I can tell, there are only 64 signals, so you don't even have to change the location of the signal mask. Furthermore, nothing in the toolchain prevents people from compiling CET-marked binaries with older glibc headers, so you can't use CET markup to determine the size of the stack allocation anyway. Thanks, Florian
On Thu, Jan 25, 2018 at 1:47 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 01/25/2018 06:33 AM, H.J. Lu wrote: > >> Please don't revert my patch. Please try this patch: >> >> >> https://sourceware.org/git/?p=glibc.git;a=commit;h=4b7fc470a6740808b41502d7431f91805e272d26 >> >> instead. I will clean it up and submit it tomorrow. > > > I don't see how adding a symbol version to pthread_create helps to solve the > general case. Callers of pthread_register_cancel and pthread_create are > often compiled at different times. Not everyone does a mass rebuild each > time they switch to a new glibc version. True. We just need to use the older struct pthread_unwind_buf when shadow stack is disabled. > I still think you are over-engineering this. The pad array has still an > unused member (the last one). Just change sigsetjmp to store the shadow > pointer in that location, then the old and new setjmp will work with the > current stack layout. As far as I can tell, there are only 64 signals, so > you don't even have to change the location of the signal mask. No, it doesn't work. struct pthread_unwind_buf is placed on caller's stack and its address is passed from applications to libpthread. If the size of caller's struct pthread_unwind_buf is smaller than what libpthread expects, libpthread will override caller's stack. > Furthermore, nothing in the toolchain prevents people from compiling That is not true. From CET spec: https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-cet.pdf On a SHSTK capable processor, the following steps should be taken: 1. When loading an executable without interpreter, enable and lock SHSTK if GNU_PROPERTY_X86_FEATURE_1_SHSTK is set on the executable. 2. When loading an executable with an interpreter, enable SHSTK if GNU_PROPERTY_X86_FEATURE_1_SHSTK is set on the interpreter. The interpreter should disable SHSTK if GNU_PROPERTY_X86_FEATURE_1_SHSTK isn’t set or any shared objects loaded via the DT_NEEDED tag, otherwise lock SHSTK. 3. After SHSTK is enabled, it is an error to load a shared object without GNU_PROPERTY_X86_FEATURE_1_SHSTK. > CET-marked binaries with older glibc headers, so you can't use CET markup to > determine the size of the stack allocation anyway. > Yes, we can. We enable shadow stack at run-time only if program and all used shared objects, including dlopened ones, are shadow stack enabled, which means that they must be compiled with GCC 8 or above and glibc 2.27 or above. Since we need to save and restore shadow stack only if shadow stack is enabled, we can safely assume that caller is compiled with smaller struct pthread_unwind_buf on stack if shadow stack isn't enabled at run-time. For callers with larger struct pthread_unwind_buf, but shadow stack isn't enabled, we just have some unused space on caller's stack. Here is the patch to do that, Andrew, please work with Debian to verify that fixes the crash. BTW, my patch is on hjl/pr22743/master branch in glibc git repo. Thanks.
On 01/25/2018 01:38 PM, H.J. Lu wrote: >> I still think you are over-engineering this. The pad array has still an >> unused member (the last one). Just change sigsetjmp to store the shadow >> pointer in that location, then the old and new setjmp will work with the >> current stack layout. As far as I can tell, there are only 64 signals, so >> you don't even have to change the location of the signal mask. > No, it doesn't work. struct pthread_unwind_buf is placed on caller's stack > and its address is passed from applications to libpthread. If the size of > caller's struct pthread_unwind_buf is smaller than what libpthread expects, > libpthread will override caller's stack. As far as I can see, ibuf->priv.pad[3] is currently unused. (sig)setjmp could save the shadow stack pointer at the right offset in jmp_buf to hit this place, then all these new conditionals wouldn't be necessary. Of course it is still a hack, but your approach is not clearer IMHO. The patch you posted is very different from the commit link you shared earlier. I still need to review it in detail. Thanks, Florian
On Thu, Jan 25, 2018 at 4:50 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 01/25/2018 01:38 PM, H.J. Lu wrote: >>> >>> I still think you are over-engineering this. The pad array has still an >>> unused member (the last one). Just change sigsetjmp to store the shadow >>> pointer in that location, then the old and new setjmp will work with the >>> current stack layout. As far as I can tell, there are only 64 signals, >>> so >>> you don't even have to change the location of the signal mask. >> >> No, it doesn't work. struct pthread_unwind_buf is placed on caller's >> stack >> and its address is passed from applications to libpthread. If the size >> of >> caller's struct pthread_unwind_buf is smaller than what libpthread >> expects, >> libpthread will override caller's stack. > > > As far as I can see, ibuf->priv.pad[3] is currently unused. (sig)setjmp > could save the shadow stack pointer at the right offset in jmp_buf to hit > this place, then all these new conditionals wouldn't be necessary. Of course > it is still a hack, but your approach is not clearer IMHO. It is a mistake to use different jmpbuf sizes in libpthread and libc. My patch is much more straightforward than what you suggested. But I can live with it if everyone thinks it is the way to go. > The patch you posted is very different from the commit link you shared > earlier. I still need to review it in detail. > Yes, I reworked the legacy cleanup buffer detection. Now it simply checks if shadow stack is enabled or not.
On Thu, Jan 25, 2018 at 8:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jan 25, 2018 at 4:50 AM, Florian Weimer <fweimer@redhat.com> wrote: >> On 01/25/2018 01:38 PM, H.J. Lu wrote: >>>> >>>> I still think you are over-engineering this. The pad array has still an >>>> unused member (the last one). Just change sigsetjmp to store the shadow >>>> pointer in that location, then the old and new setjmp will work with the >>>> current stack layout. As far as I can tell, there are only 64 signals, >>>> so >>>> you don't even have to change the location of the signal mask. >>> >>> No, it doesn't work. struct pthread_unwind_buf is placed on caller's >>> stack >>> and its address is passed from applications to libpthread. If the size >>> of >>> caller's struct pthread_unwind_buf is smaller than what libpthread >>> expects, >>> libpthread will override caller's stack. >> >> >> As far as I can see, ibuf->priv.pad[3] is currently unused. (sig)setjmp >> could save the shadow stack pointer at the right offset in jmp_buf to hit >> this place, then all these new conditionals wouldn't be necessary. Of course >> it is still a hack, but your approach is not clearer IMHO. > > It is a mistake to use different jmpbuf sizes in libpthread and libc. > My patch is much more straightforward than what you suggested. > But I can live with it if everyone thinks it is the way to go. In my opinion, the fact that you two are having this argument reinforces Carlos' position: the original patch should be reverted and we should figure out what to do in 2.28 when we're not under time pressure. HJ, do you have some concrete external reason why you must have this new feature in 2.27? If so, please tell us what it is. To me it doesn't seem urgent. Also, this incident demonstrates that we need better testing of our ABI backward compatibility guarantees. Automation for taking all of the test binaries from glibc-2.x and running them against lib*.so from glibc-2.y (y > x), or something like that. Probably Joseph is in the best position to know how hard that would be. zw
On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: > On Thu, Jan 25, 2018 at 8:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Jan 25, 2018 at 4:50 AM, Florian Weimer <fweimer@redhat.com> wrote: >>> On 01/25/2018 01:38 PM, H.J. Lu wrote: >>>>> >>>>> I still think you are over-engineering this. The pad array has still an >>>>> unused member (the last one). Just change sigsetjmp to store the shadow >>>>> pointer in that location, then the old and new setjmp will work with the >>>>> current stack layout. As far as I can tell, there are only 64 signals, >>>>> so >>>>> you don't even have to change the location of the signal mask. >>>> >>>> No, it doesn't work. struct pthread_unwind_buf is placed on caller's >>>> stack >>>> and its address is passed from applications to libpthread. If the size >>>> of >>>> caller's struct pthread_unwind_buf is smaller than what libpthread >>>> expects, >>>> libpthread will override caller's stack. >>> >>> >>> As far as I can see, ibuf->priv.pad[3] is currently unused. (sig)setjmp >>> could save the shadow stack pointer at the right offset in jmp_buf to hit >>> this place, then all these new conditionals wouldn't be necessary. Of course >>> it is still a hack, but your approach is not clearer IMHO. >> >> It is a mistake to use different jmpbuf sizes in libpthread and libc. >> My patch is much more straightforward than what you suggested. >> But I can live with it if everyone thinks it is the way to go. > > In my opinion, the fact that you two are having this argument > reinforces Carlos' position: the original patch should be reverted and > we should figure out what to do in 2.28 when we're not under time > pressure. HJ, do you have some concrete external reason why you must > have this new feature in 2.27? If so, please tell us what it is. To > me it doesn't seem urgent. My question is if we are going to fix it at all. If yes, why not 2.27. Both approaches are opaque to users. They can't tell the difference.
On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: >> In my opinion, the fact that you two are having this argument >> reinforces Carlos' position: the original patch should be reverted and >> we should figure out what to do in 2.28 when we're not under time >> pressure. HJ, do you have some concrete external reason why you must >> have this new feature in 2.27? If so, please tell us what it is. To >> me it doesn't seem urgent. > > My question is if we are going to fix it at all. If yes, why not 2.27. > Both approaches are opaque to users. They can't tell the difference. My concerns are entirely based on timing: specifically, you seem to be in a rush to squeak under the 2.27 deadline. Rushing leads to mistakes. This seems like the sort of thing that could reasonably be backported to the release branch(es) ... *after* we have calmly, without rushing, figured out the correct fix in mainline. zw
On Thu, Jan 25, 2018 at 8:22 AM, Zack Weinberg <zackw@panix.com> wrote: > On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: >>> In my opinion, the fact that you two are having this argument >>> reinforces Carlos' position: the original patch should be reverted and >>> we should figure out what to do in 2.28 when we're not under time >>> pressure. HJ, do you have some concrete external reason why you must >>> have this new feature in 2.27? If so, please tell us what it is. To >>> me it doesn't seem urgent. >> >> My question is if we are going to fix it at all. If yes, why not 2.27. >> Both approaches are opaque to users. They can't tell the difference. > > My concerns are entirely based on timing: specifically, you seem to be > in a rush to squeak under the 2.27 deadline. Rushing leads to > mistakes. The main issue for this one is testcase. Once a testcase is found, we know how to avoid the issue. > This seems like the sort of thing that could reasonably be backported > to the release branch(es) ... *after* we have calmly, without rushing, > figured out the correct fix in mainline. > I am fine with reverting my patch only on 2.27 branch, not on master.
On 01/25/2018 08:28 AM, H.J. Lu wrote: > On Thu, Jan 25, 2018 at 8:22 AM, Zack Weinberg <zackw@panix.com> wrote: >> On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: >>>> In my opinion, the fact that you two are having this argument >>>> reinforces Carlos' position: the original patch should be reverted and >>>> we should figure out what to do in 2.28 when we're not under time >>>> pressure. HJ, do you have some concrete external reason why you must >>>> have this new feature in 2.27? If so, please tell us what it is. To >>>> me it doesn't seem urgent. >>> >>> My question is if we are going to fix it at all. If yes, why not 2.27. >>> Both approaches are opaque to users. They can't tell the difference. >> >> My concerns are entirely based on timing: specifically, you seem to be >> in a rush to squeak under the 2.27 deadline. Rushing leads to >> mistakes. > > The main issue for this one is testcase. Once a testcase is found, we > know how to avoid the issue. > >> This seems like the sort of thing that could reasonably be backported >> to the release branch(es) ... *after* we have calmly, without rushing, >> figured out the correct fix in mainline. >> > > I am fine with reverting my patch only on 2.27 branch, not on master. This does not make sense. The revert on master would last for as long as you have to come up with a patch that works and everyone accepts and has consensus. You checked these patches in without consensus, and instead of waiting or pinging for review, you checked them in. For x86_64 there is no machine maintainer, it requires community consensus, the port is too important not to get serious community review. They changes had negative ABI consequences, and now you have several people interested in making sure future patches don't break ABI. You have drawn attention to this work and now you have to reach consensus on a solution for a primary architecture which is very important to all of us in the downstream distributions. More time is required to make these patches work. I see no clear argument for why this needs to be in 2.27. I will be reverting the patches in the next 8 hours.
On 01/24/2018 09:33 PM, H.J. Lu wrote: > On Wed, Jan 24, 2018 at 8:53 PM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 01/24/2018 05:48 PM, Dmitry V. Levin wrote: >>> I'm afraid by Monday it will be too late for 2.27 as we will get very >>> little testing before the release. >> Before reverting: >> >> [carlos@athas tst-cleanup1]$ /home/carlos/build/glibc/elf/ld.so --library-path /home/carlos/build/glibc:/home/carlos/build/glibc/elf:/home/carlos/build/glibc/dlfcn:/home/carlos/build/glibc/nptl ./tst-cleanup1 >> ch (3) >> ch (2) >> ch (1) >> Didn't expect signal from child: got `Segmentation fault' >> >> After reverting: >> >> [carlos@athas tst-cleanup1]$ /home/carlos/build/glibc-reverted/elf/ld.so --library-path /home/carlos/build/glibc-reverted:/home/carlos/build/glibc-reverted/elf:/home/carlos/build/glibc-reverted/dlfcn:/home/carlos/build/glibc-reverted/nptl ./tst-cleanup1 >> ch (3) >> ch (2) >> ch (1) >> >> ~~~ Commit message ~~~ >> In commit cba595c350e52194e10c0006732e1991e3d0803b and commit >> f81ddabffd76ac9dd600b02adbf3e1dac4bb10ec, ABI compatibility with >> applications was broken by increasing the size of the on-stack >> allocated __pthread_unwind_buf_t beyond the oringal size. >> Applications only have the origianl space available for >> __pthread_unwind_register, and __pthread_unwind_next to use, >> any increase in the size of __pthread_unwind_buf_t causes these >> functions to write beyond the original structure into other >> on-stack variables leading to segmentation faults in common >> applications like vlc. The only workaround is to version those >> functions which operate on the old sized objects, but this must >> happen in glibc 2.28. >> >> Thank you to Andrew Senkevich, H.J. Lu, and Aurelien Jarno, for >> submitting reports and tracking the issue down. >> >> The commit reverts the above mentioned commits and testing on >> x86_64 shows that the ABI compatibility is restored. A tst-cleanup1 >> regression test linked with an older glibc now passes when run >> with the newly built glibc. Previously a tst-cleanup1 linked with >> an older glibc would segfault when run with an affected glibc build. >> >> Tested on x86_64 with no regressions. >> >> Signed-off-by: Carlos O'Donell <carlos@redhat.com> >> ~~~ >> >> Patch attached. >> >> OK to commit? >> >> This fixes the last blocker for glibc 2.27. > > Please don't revert my patch. Please try this patch: > > https://sourceware.org/git/?p=glibc.git;a=commit;h=4b7fc470a6740808b41502d7431f91805e272d26 > > instead. I will clean it up and submit it tomorrow. This is unacceptable. It adds a new symbol version and we froze the ABI at the start of the month. You cannot work these fixes into 2.27 with a new symbol version, it must wait for 2.28.
On 01/25/2018 05:37 PM, Carlos O'Donell wrote: >> Please don't revert my patch. Please try this patch: >> >> https://sourceware.org/git/?p=glibc.git;a=commit;h=4b7fc470a6740808b41502d7431f91805e272d26 >> >> instead. I will clean it up and submit it tomorrow. > This is unacceptable. It adds a new symbol version and we froze the > ABI at the start of the month. You cannot work these fixes into 2.27 > with a new symbol version, it must wait for 2.28. Note that the actually posted patch is completely different. Thanks, Florian
On Thu, Jan 25, 2018 at 8:36 AM, Carlos O'Donell <carlos@redhat.com> wrote: > On 01/25/2018 08:28 AM, H.J. Lu wrote: >> On Thu, Jan 25, 2018 at 8:22 AM, Zack Weinberg <zackw@panix.com> wrote: >>> On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>> On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: >>>>> In my opinion, the fact that you two are having this argument >>>>> reinforces Carlos' position: the original patch should be reverted and >>>>> we should figure out what to do in 2.28 when we're not under time >>>>> pressure. HJ, do you have some concrete external reason why you must >>>>> have this new feature in 2.27? If so, please tell us what it is. To >>>>> me it doesn't seem urgent. >>>> >>>> My question is if we are going to fix it at all. If yes, why not 2.27. >>>> Both approaches are opaque to users. They can't tell the difference. >>> >>> My concerns are entirely based on timing: specifically, you seem to be >>> in a rush to squeak under the 2.27 deadline. Rushing leads to >>> mistakes. >> >> The main issue for this one is testcase. Once a testcase is found, we >> know how to avoid the issue. >> >>> This seems like the sort of thing that could reasonably be backported >>> to the release branch(es) ... *after* we have calmly, without rushing, >>> figured out the correct fix in mainline. >>> >> >> I am fine with reverting my patch only on 2.27 branch, not on master. > > This does not make sense. The revert on master would last for as long as > you have to come up with a patch that works and everyone accepts and has > consensus. We have 2 proposals, one with a patch and one without. How long should it take to make a decision? > You checked these patches in without consensus, and instead of waiting > or pinging for review, you checked them in. > > For x86_64 there is no machine maintainer, it requires community consensus, Do we need/want machine maintainers for x86-64 (i386)? > the port is too important not to get serious community review. > > They changes had negative ABI consequences, and now you have several > people interested in making sure future patches don't break ABI. There are no arguments here. > You have drawn attention to this work and now you have to reach consensus > on a solution for a primary architecture which is very important to all of > us in the downstream distributions. More time is required to make these > patches work. > > I see no clear argument for why this needs to be in 2.27. That is fine with me. > I will be reverting the patches in the next 8 hours. We need this on master so that we can work on CET support.
On 01/25/2018 08:40 AM, H.J. Lu wrote: > On Thu, Jan 25, 2018 at 8:36 AM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 01/25/2018 08:28 AM, H.J. Lu wrote: >>> On Thu, Jan 25, 2018 at 8:22 AM, Zack Weinberg <zackw@panix.com> wrote: >>>> On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>> On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: >>>>>> In my opinion, the fact that you two are having this argument >>>>>> reinforces Carlos' position: the original patch should be reverted and >>>>>> we should figure out what to do in 2.28 when we're not under time >>>>>> pressure. HJ, do you have some concrete external reason why you must >>>>>> have this new feature in 2.27? If so, please tell us what it is. To >>>>>> me it doesn't seem urgent. >>>>> >>>>> My question is if we are going to fix it at all. If yes, why not 2.27. >>>>> Both approaches are opaque to users. They can't tell the difference. >>>> >>>> My concerns are entirely based on timing: specifically, you seem to be >>>> in a rush to squeak under the 2.27 deadline. Rushing leads to >>>> mistakes. >>> >>> The main issue for this one is testcase. Once a testcase is found, we >>> know how to avoid the issue. >>> >>>> This seems like the sort of thing that could reasonably be backported >>>> to the release branch(es) ... *after* we have calmly, without rushing, >>>> figured out the correct fix in mainline. >>>> >>> >>> I am fine with reverting my patch only on 2.27 branch, not on master. >> >> This does not make sense. The revert on master would last for as long as >> you have to come up with a patch that works and everyone accepts and has >> consensus. > > We have 2 proposals, one with a patch and one without. How long > should it take to make a decision? However long it takes. Until then we revert the patches. >> You checked these patches in without consensus, and instead of waiting >> or pinging for review, you checked them in. >> >> For x86_64 there is no machine maintainer, it requires community consensus, > > Do we need/want machine maintainers for x86-64 (i386)? No. We need people to follow consensus rules, ping patches, and ask for review. Without review we are going to increase the risk of defects going in to the port. The point of review is to lower defect rates and attain better solutions. >> the port is too important not to get serious community review. >> >> They changes had negative ABI consequences, and now you have several >> people interested in making sure future patches don't break ABI. > > There are no arguments here. > >> You have drawn attention to this work and now you have to reach consensus >> on a solution for a primary architecture which is very important to all of >> us in the downstream distributions. More time is required to make these >> patches work. >> >> I see no clear argument for why this needs to be in 2.27. > > That is fine with me. > >> I will be reverting the patches in the next 8 hours. > > We need this on master so that we can work on CET support. No. You do not *need* anything on master, you can work on your own branches with all of your collaborators. Master is for work that is complete, has consensus, and is ready to be delivered into a public release for which we will promise ABI guarantees.
On 01/25/2018 05:22 PM, Zack Weinberg wrote: > This seems like the sort of thing that could reasonably be backported > to the release branch(es) ...*after* we have calmly, without rushing, > figured out the correct fix in mainline. H.J.'s approach requires that glibc 2.27 is fixed now because once people build with CET, binaries will have the CET markup but still follow the old ABI (assuming we make the ABI change subsequently). (I don't understand why this doesn't already happen when glibc 2.26 headers are used to build programs with CET compiler flags.) Thanks, Florian
On Thu, Jan 25, 2018 at 8:47 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 01/25/2018 05:22 PM, Zack Weinberg wrote: >> >> This seems like the sort of thing that could reasonably be backported >> to the release branch(es) ...*after* we have calmly, without rushing, >> figured out the correct fix in mainline. > > > H.J.'s approach requires that glibc 2.27 is fixed now because once people > build with CET, binaries will have the CET markup but still follow the old > ABI (assuming we make the ABI change subsequently). No, they won't. We haven't checked in the critical patch to turn on the CET markup yet. You can build glibc 2.27 with GCC 8. But you won't get [hjl@gnu-6 build-x86_64-linux]$ readelf -n csu/crt1.o Displaying notes found in: .note.gnu.property Owner Data size Description GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 Properties: x86 feature: IBT, SHSTK Displaying notes found in: .note.ABI-tag Owner Data size Description GNU 0x00000010 NT_GNU_ABI_TAG (ABI version tag) OS: Linux, ABI: 3.2.0 [hjl@gnu-6 build-x86_64-linux]$ Use an used padding in pthread_unwind_buf to save and restore shadow stack isn't a long term solution. What do we do if we need to save and restore another register in jmp buf 5 years from now? > (I don't understand why this doesn't already happen when glibc 2.26 headers > are used to build programs with CET compiler flags.) > See above.
On Thu, Jan 25, 2018 at 8:46 AM, Carlos O'Donell <carlos@redhat.com> wrote: > On 01/25/2018 08:40 AM, H.J. Lu wrote: >> On Thu, Jan 25, 2018 at 8:36 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>> On 01/25/2018 08:28 AM, H.J. Lu wrote: >>>> On Thu, Jan 25, 2018 at 8:22 AM, Zack Weinberg <zackw@panix.com> wrote: >>>>> On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>>> On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: >>>>>>> In my opinion, the fact that you two are having this argument >>>>>>> reinforces Carlos' position: the original patch should be reverted and >>>>>>> we should figure out what to do in 2.28 when we're not under time >>>>>>> pressure. HJ, do you have some concrete external reason why you must >>>>>>> have this new feature in 2.27? If so, please tell us what it is. To >>>>>>> me it doesn't seem urgent. >>>>>> >>>>>> My question is if we are going to fix it at all. If yes, why not 2.27. >>>>>> Both approaches are opaque to users. They can't tell the difference. >>>>> >>>>> My concerns are entirely based on timing: specifically, you seem to be >>>>> in a rush to squeak under the 2.27 deadline. Rushing leads to >>>>> mistakes. >>>> >>>> The main issue for this one is testcase. Once a testcase is found, we >>>> know how to avoid the issue. >>>> >>>>> This seems like the sort of thing that could reasonably be backported >>>>> to the release branch(es) ... *after* we have calmly, without rushing, >>>>> figured out the correct fix in mainline. >>>>> >>>> >>>> I am fine with reverting my patch only on 2.27 branch, not on master. >>> >>> This does not make sense. The revert on master would last for as long as >>> you have to come up with a patch that works and everyone accepts and has >>> consensus. >> >> We have 2 proposals, one with a patch and one without. How long >> should it take to make a decision? > > However long it takes. > > Until then we revert the patches. > Sure. Please revert it now. I will submit a patch to re-apply it + my fix after 2.27 branch is taken.
On Thu, 25 Jan 2018, Zack Weinberg wrote: > Also, this incident demonstrates that we need better testing of our > ABI backward compatibility guarantees. Automation for taking all of > the test binaries from glibc-2.x and running them against lib*.so from > glibc-2.y (y > x), or something like that. Probably Joseph is in the > best position to know how hard that would be. I'd suggest, in the 2.x build tree, make check rtld-prefix=SOMETHING run-program-env=SOMETHING where the SOMETHINGs are appropriately defined to use the build or install trees of 2.y. There are probably reasons that doesn't work for some or all tests (apart from deliberate changes in glibc that required testsuite changes but weren't considered to require new symbol versions) - but that should be the basic idea of how to test binaries using a newer glibc version at runtime. (We can make changes to glibc to make that sort of thing easier if it turns out such changes would help, but still want to be able to do it when x, the old version, is a glibc version without those changes.)
On Thu, Jan 25, 2018 at 4:50 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 01/25/2018 01:38 PM, H.J. Lu wrote: >>> >>> I still think you are over-engineering this. The pad array has still an >>> unused member (the last one). Just change sigsetjmp to store the shadow >>> pointer in that location, then the old and new setjmp will work with the >>> current stack layout. As far as I can tell, there are only 64 signals, >>> so >>> you don't even have to change the location of the signal mask. >> >> No, it doesn't work. struct pthread_unwind_buf is placed on caller's >> stack >> and its address is passed from applications to libpthread. If the size >> of >> caller's struct pthread_unwind_buf is smaller than what libpthread >> expects, >> libpthread will override caller's stack. > > > As far as I can see, ibuf->priv.pad[3] is currently unused. (sig)setjmp > could save the shadow stack pointer at the right offset in jmp_buf to hit > this place, then all these new conditionals wouldn't be necessary. Of course > it is still a hack, but your approach is not clearer IMHO. FWIW, this approach doesn't work with x32 which is 64-bit process with 32-bit software pointer. Kernel may place shadow stack above 4GB. We need to save and restore 64-bit shadow stack register for x32.
On 01/25/2018 09:01 AM, H.J. Lu wrote: > On Thu, Jan 25, 2018 at 8:46 AM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 01/25/2018 08:40 AM, H.J. Lu wrote: >>> On Thu, Jan 25, 2018 at 8:36 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>>> On 01/25/2018 08:28 AM, H.J. Lu wrote: >>>>> On Thu, Jan 25, 2018 at 8:22 AM, Zack Weinberg <zackw@panix.com> wrote: >>>>>> On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>>>> On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: >>>>>>>> In my opinion, the fact that you two are having this argument >>>>>>>> reinforces Carlos' position: the original patch should be reverted and >>>>>>>> we should figure out what to do in 2.28 when we're not under time >>>>>>>> pressure. HJ, do you have some concrete external reason why you must >>>>>>>> have this new feature in 2.27? If so, please tell us what it is. To >>>>>>>> me it doesn't seem urgent. >>>>>>> >>>>>>> My question is if we are going to fix it at all. If yes, why not 2.27. >>>>>>> Both approaches are opaque to users. They can't tell the difference. >>>>>> >>>>>> My concerns are entirely based on timing: specifically, you seem to be >>>>>> in a rush to squeak under the 2.27 deadline. Rushing leads to >>>>>> mistakes. >>>>> >>>>> The main issue for this one is testcase. Once a testcase is found, we >>>>> know how to avoid the issue. >>>>> >>>>>> This seems like the sort of thing that could reasonably be backported >>>>>> to the release branch(es) ... *after* we have calmly, without rushing, >>>>>> figured out the correct fix in mainline. >>>>>> >>>>> >>>>> I am fine with reverting my patch only on 2.27 branch, not on master. >>>> >>>> This does not make sense. The revert on master would last for as long as >>>> you have to come up with a patch that works and everyone accepts and has >>>> consensus. >>> >>> We have 2 proposals, one with a patch and one without. How long >>> should it take to make a decision? >> >> However long it takes. >> >> Until then we revert the patches. >> > > Sure. Please revert it now. > > I will submit a patch to re-apply it + my fix after 2.27 branch > is taken. I have reverted the ABI breaking changes along with the matching change which adds feature_1. Thank you for working with everyone on this issue. I will spend the day tomorrow reviewing these patches.
On Thu, Jan 25, 2018 at 11:45 PM, Carlos O'Donell <carlos@redhat.com> wrote: > On 01/25/2018 09:01 AM, H.J. Lu wrote: >> On Thu, Jan 25, 2018 at 8:46 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>> On 01/25/2018 08:40 AM, H.J. Lu wrote: >>>> On Thu, Jan 25, 2018 at 8:36 AM, Carlos O'Donell <carlos@redhat.com> wrote: >>>>> On 01/25/2018 08:28 AM, H.J. Lu wrote: >>>>>> On Thu, Jan 25, 2018 at 8:22 AM, Zack Weinberg <zackw@panix.com> wrote: >>>>>>> On Thu, Jan 25, 2018 at 10:33 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>>>>> On Thu, Jan 25, 2018 at 6:55 AM, Zack Weinberg <zackw@panix.com> wrote: >>>>>>>>> In my opinion, the fact that you two are having this argument >>>>>>>>> reinforces Carlos' position: the original patch should be reverted and >>>>>>>>> we should figure out what to do in 2.28 when we're not under time >>>>>>>>> pressure. HJ, do you have some concrete external reason why you must >>>>>>>>> have this new feature in 2.27? If so, please tell us what it is. To >>>>>>>>> me it doesn't seem urgent. >>>>>>>> >>>>>>>> My question is if we are going to fix it at all. If yes, why not 2.27. >>>>>>>> Both approaches are opaque to users. They can't tell the difference. >>>>>>> >>>>>>> My concerns are entirely based on timing: specifically, you seem to be >>>>>>> in a rush to squeak under the 2.27 deadline. Rushing leads to >>>>>>> mistakes. >>>>>> >>>>>> The main issue for this one is testcase. Once a testcase is found, we >>>>>> know how to avoid the issue. >>>>>> >>>>>>> This seems like the sort of thing that could reasonably be backported >>>>>>> to the release branch(es) ... *after* we have calmly, without rushing, >>>>>>> figured out the correct fix in mainline. >>>>>>> >>>>>> >>>>>> I am fine with reverting my patch only on 2.27 branch, not on master. >>>>> >>>>> This does not make sense. The revert on master would last for as long as >>>>> you have to come up with a patch that works and everyone accepts and has >>>>> consensus. >>>> >>>> We have 2 proposals, one with a patch and one without. How long >>>> should it take to make a decision? >>> >>> However long it takes. >>> >>> Until then we revert the patches. >>> >> >> Sure. Please revert it now. >> >> I will submit a patch to re-apply it + my fix after 2.27 branch >> is taken. > > I have reverted the ABI breaking changes along with the matching > change which adds feature_1. > > Thank you for working with everyone on this issue. I will spend the > day tomorrow reviewing these patches. > FYI, my current patch is on hjl/pr22743/master branch at https://github.com/hjl-tools/glibc/tree/hjl/pr22743/master
From 440f414842b61923dc8219b26df10d2a12de0f82 Mon Sep 17 00:00:00 2001 From: Carlos O'Donell <carlos@systemhalted.org> Date: Wed, 24 Jan 2018 20:35:22 -0800 Subject: [PATCH] Revert Intel CET changes to __jmp_buf_tag (Bug 22743) In commit cba595c350e52194e10c0006732e1991e3d0803b and commit f81ddabffd76ac9dd600b02adbf3e1dac4bb10ec, ABI compatibility with applications was broken by increasing the size of the on-stack allocated __pthread_unwind_buf_t beyond the oringal size. Applications only have the origianl space available for __pthread_unwind_register, and __pthread_unwind_next to use, any increase in the size of __pthread_unwind_buf_t causes these functions to write beyond the original structure into other on-stack variables leading to segmentation faults in common applications like vlc. The only workaround is to version those functions which operate on the old sized objects, but this must happen in glibc 2.28. Thank you to Andrew Senkevich, H.J. Lu, and Aurelien Jarno, for submitting reports and tracking the issue down. The commit reverts the above mentioned commits and testing on x86_64 shows that the ABI compatibility is restored. A tst-cleanup1 regression test linked with an older glibc now passes when run with the newly built glibc. Previously a tst-cleanup1 linked with an older glibc would segfault when run with an affected glibc build. Tested on x86_64 with no regressions. Signed-off-by: Carlos O'Donell <carlos@redhat.com> --- ChangeLog | 30 ++++++++++++++++++ bits/types/__cancel_jmp_buf_tag.h | 28 ----------------- nptl/Makefile | 3 +- nptl/descr.h | 3 -- sysdeps/i386/nptl/tcb-offsets.sym | 1 - sysdeps/i386/nptl/tls.h | 4 --- sysdeps/nptl/pthread.h | 7 +++-- sysdeps/unix/sysv/linux/hppa/pthread.h | 7 +++-- .../linux/x86/bits/types/__cancel_jmp_buf_tag.h | 31 ------------------- sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h | 36 ---------------------- sysdeps/unix/sysv/linux/x86/pthreaddef.h | 22 ------------- sysdeps/x86_64/nptl/tcb-offsets.sym | 1 - sysdeps/x86_64/nptl/tls.h | 5 +-- 13 files changed, 42 insertions(+), 136 deletions(-) delete mode 100644 bits/types/__cancel_jmp_buf_tag.h delete mode 100644 sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h delete mode 100644 sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h delete mode 100644 sysdeps/unix/sysv/linux/x86/pthreaddef.h diff --git a/ChangeLog b/ChangeLog index 002839213f..fb287d8e75 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,33 @@ +2018-01-24 Carlos O'Donll <carlos@redhat.com> + + Revert: + + 2017-12-19 H.J. Lu <hongjiu.lu@intel.com> + + [BZ #22563] + * sysdeps/i386/nptl/tcb-offsets.sym (FEATURE_1_OFFSET): New. + * sysdeps/i386/nptl/tls.h (tcbhead_t): Add feature_1. + * sysdeps/x86_64/nptl/tcb-offsets.sym (FEATURE_1_OFFSET): New. + * sysdeps/x86_64/nptl/tls.h (tcbhead_t): Rename __glibc_unused1 + to feature_1. + + 2017-12-19 H.J. Lu <hongjiu.lu@intel.com> + + [BZ #22563] + * bits/types/__cancel_jmp_buf_tag.h: New file. + * sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h + * sysdeps/unix/sysv/linux/x86/pthreaddef.h: Likewise. + * sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h: Likewise. + * nptl/Makefile (headers): Add + bits/types/__cancel_jmp_buf_tag.h. + * nptl/descr.h [NEED_SAVED_MASK_IN_CANCEL_JMP_BUF] + (pthread_unwind_buf): Add saved_mask to cancel_jmp_buf. + * sysdeps/nptl/pthread.h: Include + <bits/types/__cancel_jmp_buf_tag.h>. + (__pthread_unwind_buf_t): Use struct __cancel_jmp_buf_tag with + __cancel_jmp_buf. + * sysdeps/unix/sysv/linux/hppa/pthread.h: Likewise. + 2018-01-24 Joseph Myers <joseph@codesourcery.com> * sysdeps/unix/sysv/linux/m68k/jmp_buf-macros.h: Move to .... diff --git a/bits/types/__cancel_jmp_buf_tag.h b/bits/types/__cancel_jmp_buf_tag.h deleted file mode 100644 index 62f5c61f83..0000000000 --- a/bits/types/__cancel_jmp_buf_tag.h +++ /dev/null @@ -1,28 +0,0 @@ -/* Define struct __cancel_jmp_buf_tag. - Copyright (C) 2017-2018 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <http://www.gnu.org/licenses/>. */ - -#ifndef ____cancel_jmp_buf_tag_defined -#define ____cancel_jmp_buf_tag_defined 1 - -struct __cancel_jmp_buf_tag - { - __jmp_buf __cancel_jmp_buf; - int __mask_was_saved; - }; - -#endif diff --git a/nptl/Makefile b/nptl/Makefile index 7940b3d26b..6fc2c8bb6a 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -22,8 +22,7 @@ subdir := nptl include ../Makeconfig -headers := pthread.h semaphore.h bits/semaphore.h \ - bits/types/__cancel_jmp_buf_tag.h +headers := pthread.h semaphore.h bits/semaphore.h extra-libs := libpthread extra-libs-others := $(extra-libs) diff --git a/nptl/descr.h b/nptl/descr.h index 1cc6b09d1e..64ba29e1cb 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -65,9 +65,6 @@ struct pthread_unwind_buf { __jmp_buf jmp_buf; int mask_was_saved; -#ifdef NEED_SAVED_MASK_IN_CANCEL_JMP_BUF - __sigset_t saved_mask; -#endif } cancel_jmp_buf[1]; union diff --git a/sysdeps/i386/nptl/tcb-offsets.sym b/sysdeps/i386/nptl/tcb-offsets.sym index 250f1a6e13..695a810386 100644 --- a/sysdeps/i386/nptl/tcb-offsets.sym +++ b/sysdeps/i386/nptl/tcb-offsets.sym @@ -15,4 +15,3 @@ POINTER_GUARD offsetof (tcbhead_t, pointer_guard) #ifndef __ASSUME_PRIVATE_FUTEX PRIVATE_FUTEX offsetof (tcbhead_t, private_futex) #endif -FEATURE_1_OFFSET offsetof (tcbhead_t, feature_1) diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h index 30643d452a..fcda135b7c 100644 --- a/sysdeps/i386/nptl/tls.h +++ b/sysdeps/i386/nptl/tls.h @@ -50,10 +50,6 @@ typedef struct void *__private_tm[4]; /* GCC split stack support. */ void *__private_ss; - /* Bit 0: IBT. - Bit 1: SHSTK. - */ - unsigned int feature_1; } tcbhead_t; # define TLS_MULTIPLE_THREADS_IN_TCB 1 diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h index c8ba5a75c5..df049abf74 100644 --- a/sysdeps/nptl/pthread.h +++ b/sysdeps/nptl/pthread.h @@ -27,7 +27,6 @@ #include <bits/setjmp.h> #include <bits/wordsize.h> #include <bits/types/struct_timespec.h> -#include <bits/types/__cancel_jmp_buf_tag.h> /* Detach state. */ @@ -524,7 +523,11 @@ extern void pthread_testcancel (void); typedef struct { - struct __cancel_jmp_buf_tag __cancel_jmp_buf[1]; + struct + { + __jmp_buf __cancel_jmp_buf; + int __mask_was_saved; + } __cancel_jmp_buf[1]; void *__pad[4]; } __pthread_unwind_buf_t __attribute__ ((__aligned__)); diff --git a/sysdeps/unix/sysv/linux/hppa/pthread.h b/sysdeps/unix/sysv/linux/hppa/pthread.h index 3df5e7c2ac..11a024db59 100644 --- a/sysdeps/unix/sysv/linux/hppa/pthread.h +++ b/sysdeps/unix/sysv/linux/hppa/pthread.h @@ -27,7 +27,6 @@ #include <bits/setjmp.h> #include <bits/wordsize.h> #include <bits/types/struct_timespec.h> -#include <bits/types/__cancel_jmp_buf_tag.h> /* Detach state. */ @@ -500,7 +499,11 @@ extern void pthread_testcancel (void); typedef struct { - struct __cancel_jmp_buf_tag __cancel_jmp_buf[1]; + struct + { + __jmp_buf __cancel_jmp_buf; + int __mask_was_saved; + } __cancel_jmp_buf[1]; void *__pad[4]; } __pthread_unwind_buf_t __attribute__ ((__aligned__)); diff --git a/sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h b/sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h deleted file mode 100644 index 70efbb190c..0000000000 --- a/sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h +++ /dev/null @@ -1,31 +0,0 @@ -/* Define struct __cancel_jmp_buf_tag. - Copyright (C) 2017-2018 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <http://www.gnu.org/licenses/>. */ - -#ifndef ____cancel_jmp_buf_tag_defined -#define ____cancel_jmp_buf_tag_defined 1 - -#include <bits/types/__sigset_t.h> - -struct __cancel_jmp_buf_tag - { - __jmp_buf __cancel_jmp_buf; - int __mask_was_saved; - __sigset_t __saved_mask; - }; - -#endif diff --git a/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h b/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h deleted file mode 100644 index 247a62e9a0..0000000000 --- a/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h +++ /dev/null @@ -1,36 +0,0 @@ -/* Internal pthread header. Linux/x86 version. - Copyright (C) 2017-2018 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <http://www.gnu.org/licenses/>. */ - -#include_next <nptl/pthreadP.h> - -#ifndef _PTHREADP_H_X86 -#define _PTHREADP_H_X86 1 - -extern struct pthread_unwind_buf ____pthread_unwind_buf_private; - -_Static_assert (sizeof (____pthread_unwind_buf_private.cancel_jmp_buf) - >= sizeof (struct __jmp_buf_tag), - "size of cancel_jmp_buf < sizeof __jmp_buf_tag"); - -extern __pthread_unwind_buf_t ____pthread_unwind_buf; - -_Static_assert (sizeof (____pthread_unwind_buf.__cancel_jmp_buf) - >= sizeof (struct __jmp_buf_tag), - "size of __cancel_jmp_buf < sizeof __jmp_buf_tag"); - -#endif diff --git a/sysdeps/unix/sysv/linux/x86/pthreaddef.h b/sysdeps/unix/sysv/linux/x86/pthreaddef.h deleted file mode 100644 index a405a65666..0000000000 --- a/sysdeps/unix/sysv/linux/x86/pthreaddef.h +++ /dev/null @@ -1,22 +0,0 @@ -/* Pthread macros. Linux/x86 version. - Copyright (C) 2017-2018 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <http://www.gnu.org/licenses/>. */ - -#include_next <pthreaddef.h> - -/* Need saved_mask in cancel_jmp_buf. */ -#define NEED_SAVED_MASK_IN_CANCEL_JMP_BUF 1 diff --git a/sysdeps/x86_64/nptl/tcb-offsets.sym b/sysdeps/x86_64/nptl/tcb-offsets.sym index 03b6dba5c3..8a25c482cb 100644 --- a/sysdeps/x86_64/nptl/tcb-offsets.sym +++ b/sysdeps/x86_64/nptl/tcb-offsets.sym @@ -15,7 +15,6 @@ VGETCPU_CACHE_OFFSET offsetof (tcbhead_t, vgetcpu_cache) #ifndef __ASSUME_PRIVATE_FUTEX PRIVATE_FUTEX offsetof (tcbhead_t, private_futex) #endif -FEATURE_1_OFFSET offsetof (tcbhead_t, feature_1) -- Not strictly offsets, but these values are also used in the TCB. TCB_CANCELSTATE_BITMASK CANCELSTATE_BITMASK diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h index 7f0b292f42..bdd02376f9 100644 --- a/sysdeps/x86_64/nptl/tls.h +++ b/sysdeps/x86_64/nptl/tls.h @@ -56,10 +56,7 @@ typedef struct # else int __glibc_reserved1; # endif - /* Bit 0: IBT. - Bit 1: SHSTK. - */ - unsigned int feature_1; + int __glibc_unused1; /* Reservation of some values for the TM ABI. */ void *__private_tm[4]; /* GCC split stack support. */ -- 2.14.3