Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
On Sun, Oct 31, 2021 at 7:36 PM Fāng-ruì Sòng wrote: > > On Fri, Oct 8, 2021 at 10:57 AM Fāng-ruì Sòng wrote: > > > > On Fri, Sep 24, 2021 at 11:29 AM H.J. Lu wrote: > > > > > > On Fri, Sep 24, 2021 at 11:14 AM Fāng-ruì Sòng wrote: > > > > > > > > On Fri, Sep 24, 2021 at 10:41 AM H.J. Lu wrote: > > > > > > > > > > On Fri, Sep 24, 2021 at 10:29 AM Fāng-ruì Sòng > > > > > wrote: > > > > > > > > > > > > On Tue, Sep 21, 2021 at 7:08 PM Fāng-ruì Sòng > > > > > > wrote: > > > > > > > > > > > > > > On Tue, Sep 21, 2021 at 6:57 PM H.J. Lu > > > > > > > wrote: > > > > > > > > > > > > > > > > On Tue, Sep 21, 2021 at 9:16 AM Uros Bizjak > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Mon, Sep 20, 2021 at 8:20 PM Fāng-ruì Sòng via Gcc-patches > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > PING^5 > > > > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > PING^4 > > > > > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > > > > > > > > > > > > > One major design goal of PIE was to avoid copy > > > > > > > > > > > relocations. > > > > > > > > > > > The original patch for GCC 5 caused problems for many > > > > > > > > > > > years. > > > > > > > > > > > > > > > > > > > > > > On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng > > > > > > > > > > > wrote: > > > > > > > > > > >> > > > > > > > > > > >> PING^3 > > > > > > > > > > >> https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > >> > > > > > > > > > > >> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng > > > > > > > > > > >> wrote: > > > > > > > > > > >> > > > > > > > > > > > >> > PING^2 > > > > > > > > > > >> > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > >> > > > > > > > > > > > >> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng > > > > > > > > > > >> > wrote: > > > > > > > > > > >> > > > > > > > > > > > > >> > > Ping > > > > > > > > > > >> > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > >> > > > > > > > > > > > > >> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song > > > > > > > > > > >> > > wrote: > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > This was introduced in 2014-12 to use local > > > > > > > > > > >> > > > binding for external symbols > > > > > > > > > > >> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX > > > > > > > > > > >> > > > for years which mostly > > > > > > > > > > >> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, > > > > > > > > > > >> > > > HAVE_LD_PIE_COPYRELOC > > > > > > > > > > >> > > > should retire now. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > One design goal of -fPIE was to avoid co
Re: [PATCH] options: Make -Ofast switch off -fsemantic-interposition
On Fri, Nov 19, 2021 at 9:55 AM Martin Jambor wrote: > > Hi, > > On Fri, Nov 19 2021, Jan Hubicka wrote: > >> > Hi, > >> > > >> > On Fri, Nov 12 2021, Martin Jambor wrote: > >> > > Hi, > >> > > > >> > > using -fno-semantic-interposition has been reported by various people > >> > > to bring about considerable speed up at the cost of strict compliance > >> > > to the ELF symbol interposition rules See for example > >> > > https://fedoraproject.org/wiki/Changes/PythonNoSemanticInterpositionSpeedup > >> > > > >> > > As such I believe it should be implied by our -Ofast optimization > >> > > level, not only so that benchmarks that can benefit run faster, but > >> > > also so that people looking at -Ofast documentation for options that > >> > > could speed their programs find it. > >> > > > >> > > I have verified that with the following patch IPA-CP sees > >> > > flag_semantic_interposition set to zero at Ofast and that info and pdf > >> > > manual builds fine with the documentation change. I am bootstrapping > >> > > and testing it now in order to comply with submission criteria but I > >> > > don't think an Ofast change gets much tested. > >> > > > >> > > Assuming it passes, is the patch OK? (If it is, I will also add a note > >> > > about it in the "Caveats" section in gcc-12/changes.html of wwwdocs > >> > > after I commit the patch.) > >> > > > >> > > >> > Unfortunately, I was wrong, there are testcases which use the optimize > >> > attribute to switch a function to Ofast and those ICE because > >> > -fsemantic-interposition is not an optimization flag and only > >> > optimization flags can change in an optimize attribute (AFAIK, I only > >> > had a quick glance at the results). > >> > > >> > I am not sure what is the right way to tackle this, whether to set the > >> > flag at Ofast in some nonstandard way or make the flag an optimization > >> > flag - probably affecting function definitions, having it affect > >> > call-sites seems too fine-grained. I will try to discuss this on IRC on > >> > Monday (and hope such change is still doable early stage3). > >> > > >> > Sorry for posting this a bit prematurely, > >> > >> Hi, > >> > >> This patch turns flag_semantic_interposition to optimization option so > >> it can be enabled with per-function granuality. This is done by adding > >> the flag among visibility flags into the symbol table. This fixes the > >> behaviour on the testcase I added to testsuite. > >> > >> There are bugs where get_availability misbehaves on partitioned program. > >> We can also use the new flag to fix those, but I will do that > >> incrementally. > >> > >> The -Ofast change should be safe now. > > > > Also forgot to say it explicitly, the patch is OK, so please commit it. > > > > Thanks a lot for the enabling patch. I have committed mine after re-testing. > > Martin Perhaps reconsider https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100937 (configure: Add --enable-default-semantic-interposition)? Patch: https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572018.html I happen a write-up on https://maskray.me/blog/2021-05-09-fno-semantic-interposition -- 宋方睿
Re: [PATCH v4] x86: Add -m[no-]direct-extern-access
On Tue, Feb 8, 2022 at 7:05 PM H.J. Lu via Gcc-patches wrote: > > On Tue, Feb 8, 2022 at 6:38 PM Hongtao Liu wrote: > > > > On Fri, Jan 28, 2022 at 5:53 AM H.J. Lu via Gcc-patches > > wrote: > > > > > > The v3 patch was posted at > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-July/574847.html > > > > > > There is no progress with repeated pings since then. Glibc 2.35 and > > > binutils 2.38 will support GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS. > > > I'd like to remove copy relocation to improve security and performance > > > on x86 in GCC 12. Here is the v4 patch > > This patch won't change default behavior for copy relocation(to > > avoid conflict with existing .so files), > > just give users an option under which copy relocation can be removed. > > The removal of copy relocation is an optimization of the linker(Also > > improves security), whose > > patches have been approved and committed to glibc2.35[1], binutils2.38[2]. > > The compiler part is the final step in completing this optimization. > > Thanks for looking into it. Since the new behavior is off by default and > none of the maintainers feel comfortable to review my patch, which is > related to linker and glibc, for the last 6 months, now is the good time > to check it in as any other time. I will check it in tomorrow. Shall we move forward with "x86-64: Remove HAVE_LD_PIE_COPYRELOC" (https://gcc.gnu.org/pipermail/gcc-patches/2021-November/582987.html)? It's more about the correct behavior for -fpie. The -mno-direct-extern-access transition will certainly take a long time, but we should not let that block fixing the GCC 5 x86-64 regression. > > [1] https://sourceware.org/pipermail/libc-alpha/2021-October/131742.html > > [2] https://sourceware.org/pipermail/binutils/2021-July/117308.html > > > > > > 1. Rename the common option to x86 specific -mdirect-extern-access option. > > > 2. Remove GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS linker check. > > > 3. Use the existing GNU property function in x86 backend. > > > > > > This new behavior is off by default. Are there any objections? > > > > > > Changes in the v3 patch. > > > > > > 1. GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS support has been added to > > > GNU binutils 2.38. But the -z indirect-extern-access linker option is > > > only available for Linux/x86. However, the --max-cache-size=SIZE linker > > > option was also addded within a day. --max-cache-size=SIZE is used to > > > check for GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS support. > > > > > > Changes in the v2 patch. > > > > > > 1. Rename the option to -fdirect-extern-access. > > > > > > --- > > > On systems with copy relocation: > > > * A copy in executable is created for the definition in a shared library > > > at run-time by ld.so. > > > * The copy is referenced by executable and shared libraries. > > > * Executable can access the copy directly. > > > > > > Issues are: > > > * Overhead of a copy, time and space, may be visible at run-time. > > > * Read-only data in the shared library becomes read-write copy in > > > executable at run-time. > > > * Local access to data with the STV_PROTECTED visibility in the shared > > > library must use GOT. > > > > > > On systems without function descriptor, function pointers vary depending > > > on where and how the functions are defined. > > > * If the function is defined in executable, it can be the address of > > > function body. > > > * If the function, including the function with STV_PROTECTED visibility, > > > is defined in the shared library, it can be the address of the PLT entry > > > in executable or shared library. > > > > > > Issues are: > > > * The address of function body may not be used as its function pointer. > > > * ld.so needs to search loaded shared libraries for the function pointer > > > of the function with STV_PROTECTED visibility. > > > > > > Here is a proposal to remove copy relocation and use canonical function > > > pointer: > > > > > > 1. Accesses, including in PIE and non-PIE, to undefined symbols must > > > use GOT. > > > a. Linker may optimize out GOT access if the data is defined in PIE or > > > non-PIE. > > > 2. Read-only data in the shared library remain read-only at run-time > > > 3. Address of global data with the STV_PROTECTED visibility in the shared > > > library is the address of data body. > > > a. Can use IP-relative access. > > > b. May need GOT without IP-relative access. > > > 4. For systems without function descriptor, > > > a. All global function pointers of undefined functions in PIE and > > > non-PIE must use GOT. Linker may optimize out GOT access if the > > > function is defined in PIE or non-PIE. > > > b. Function pointer of functions with the STV_PROTECTED visibility in > > > executable and shared library is the address of function body. > > >i. Can use IP-relative access. > > >ii. May need GOT without IP-relative access. > > >iii. Branches to undefined functions may
Re: PING^5 [PATCH v4 0/2] Implement indirect external access
On Mon, Jan 3, 2022 at 7:33 PM H.J. Lu via Gcc-patches wrote: > > On Sat, Dec 11, 2021 at 10:44 AM H.J. Lu wrote: > > > > On Thu, Nov 25, 2021 at 9:54 AM H.J. Lu wrote: > > > > > > On Mon, Nov 1, 2021 at 7:02 AM H.J. Lu wrote: > > > > > > > > On Thu, Oct 21, 2021 at 12:56 PM H.J. Lu wrote: > > > > > > > > > > On Wed, Sep 22, 2021 at 7:02 PM H.J. Lu wrote: > > > > > > > > > > > > Changes in the v4 patch. > > > > > > > > > > > > 1. Add nodirect_extern_access attribute. > > > > > > > > > > > > Changes in the v3 patch. > > > > > > > > > > > > 1. GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS support has been > > > > > > added to > > > > > > GNU binutils 2.38. But the -z indirect-extern-access linker option > > > > > > is > > > > > > only available for Linux/x86. However, the --max-cache-size=SIZE > > > > > > linker > > > > > > option was also addded within a day. --max-cache-size=SIZE is used > > > > > > to > > > > > > check for GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS support. > > > > > > > > > > > > Changes in the v2 patch. > > > > > > > > > > > > 1. Rename the option to -fdirect-extern-access. > > > > > > > > > > > > --- > > > > > > On systems with copy relocation: > > > > > > * A copy in executable is created for the definition in a shared > > > > > > library > > > > > > at run-time by ld.so. > > > > > > * The copy is referenced by executable and shared libraries. > > > > > > * Executable can access the copy directly. > > > > > > > > > > > > Issues are: > > > > > > * Overhead of a copy, time and space, may be visible at run-time. > > > > > > * Read-only data in the shared library becomes read-write copy in > > > > > > executable at run-time. > > > > > > * Local access to data with the STV_PROTECTED visibility in the > > > > > > shared > > > > > > library must use GOT. > > > > > > > > > > > > On systems without function descriptor, function pointers vary > > > > > > depending > > > > > > on where and how the functions are defined. > > > > > > * If the function is defined in executable, it can be the address of > > > > > > function body. > > > > > > * If the function, including the function with STV_PROTECTED > > > > > > visibility, > > > > > > is defined in the shared library, it can be the address of the PLT > > > > > > entry > > > > > > in executable or shared library. > > > > > > > > > > > > Issues are: > > > > > > * The address of function body may not be used as its function > > > > > > pointer. > > > > > > * ld.so needs to search loaded shared libraries for the function > > > > > > pointer > > > > > > of the function with STV_PROTECTED visibility. > > > > > > > > > > > > Here is a proposal to remove copy relocation and use canonical > > > > > > function > > > > > > pointer: > > > > > > > > > > > > 1. Accesses, including in PIE and non-PIE, to undefined symbols must > > > > > > use GOT. > > > > > > a. Linker may optimize out GOT access if the data is defined in > > > > > > PIE or > > > > > > non-PIE. > > > > > > 2. Read-only data in the shared library remain read-only at run-time > > > > > > 3. Address of global data with the STV_PROTECTED visibility in the > > > > > > shared > > > > > > library is the address of data body. > > > > > > a. Can use IP-relative access. > > > > > > b. May need GOT without IP-relative access. > > > > > > 4. For systems without function descriptor, > > > > > > a. All global function pointers of undefined functions in PIE and > > > > > > non-PIE must use GOT. Linker may optimize out GOT access if the > > > > > > function is defined in PIE or non-PIE. > > > > > > b. Function pointer of functions with the STV_PROTECTED > > > > > > visibility in > > > > > > executable and shared library is the address of function body. > > > > > >i. Can use IP-relative access. > > > > > >ii. May need GOT without IP-relative access. > > > > > >iii. Branches to undefined functions may use PLT. > > > > > > 5. Single global definition marker: > > > > > > > > > > > > Add GNU_PROPERTY_1_NEEDED: > > > > > > > > > > > > #define GNU_PROPERTY_1_NEEDED GNU_PROPERTY_UINT32_OR_LO > > > > > > > > > > > > to indicate the needed properties by the object file. > > > > > > > > > > > > Add GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS: > > > > > > > > > > > > #define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0) > > > > > > > > > > > > to indicate that the object file requires canonical function > > > > > > pointers and > > > > > > cannot be used with copy relocation. This bit should be cleared in > > > > > > executable when there are non-GOT or non-PLT relocations in > > > > > > relocatable > > > > > > input files without this bit set. > > > > > > > > > > > > a. Protected symbol access within the shared library can be > > > > > > treated as > > > > > > local. > > > > > > b. Copy relocation should be disallowed at link-time and run-time. > > > > > > c. GOT function pointer reference is required at link-time and > > > > > > run-time. > >
Re: [PATCH] elf: Add __libc_get_static_tls_bounds [BZ #16291]
On Mon, Nov 29, 2021 at 11:30 AM Florian Weimer wrote: > > * Fāng-ruì Sòng: > > > PING^3 > > I think the core issue with this patch is like this: > > * I do not want to commit glibc to a public API that disallows future > changes to the way we allocate static TLS. While static TLS objects > cannot move in memory, the extent of the static TLS area (minimum and > maximum address) is not fixed by ABI necessities. > > * Joseph does not want to add a GLIBC_PRIVATE ABI that is exclusively > used externally. > > I have tried repeatly to wrap my head around how the sanitizers use the > static TLS boundary information. Based on what I can tell, there are > several applications: > > 1. Thead Sanitizer needs to know application-accessible thread-specific >memory that is carried via the glibc thread (stack) cache from one >thread to another one, seemingly without synchronization. Since the >synchronization happens internally within glibc, without that extra >knowledge, Thread Sanitizer would report a false positive. This >covers only data to which the application has direct access, internal >access by glibc does not count because it is not going to be >instrumented. > > 2. Address Sanitizer needs TLS boundary information for bounds checking. >Again this only applies to accesses that can be instrumented, so only >objects whose addresses leak to application code count. (Maybe this >is a fringe use case, though: it seems to apply only to “extern >__thread int a[];“ and similar declarations, where the declared type >is not enough.) > > 3. Leak Sanitizer needs to find all per-thread pointers pointing into >the malloc heap, within memory not itself allocated by malloc. This >includes the DTV, pthread_getspecific metadata, and perhaps also user >data set by pthread_getspecific, and of course static TLS variables. >This goes someone beyond what people would usually consider static >TLS: the DTV and struct pthread are not really part of static TLS as >such. And struct pthread has several members that contain malloc'ed >pointers. > > Is this a complete list of uses? Perhaps add HWAddressSanitizer along with Address Sanitizer and Memory Sanitizer along with Thread Sanitizer. Then I think the list is complete. > I *think* you can get what you need via existing GLIBC_PRIVATE > interfaces. But in order to describe how to caox the data out of glibc, > I need to know what you need. Unfortunate no, not reliably. Currently _dl_get_tls_static_info (https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp#L207) is used. It has the size information but not the start address, so the sanitizer runtime is doing very ugly/error-prone probing of the start address by reading the thread pointer and hard coding the `struct pthread` size for various glibc versions. Every time `struct pthreads` increases in glibc, sanitizer runtime has to adapt. __libc_get_static_tls_bounds if supported by glibc, can replace _dl_get_tls_static_info and the existing glibc specific GetTls hacks, and provide a bit more compatibility when glibc struct pthreads increases. > (Cc:ing a few people from a recent GCC thread.) > > Thanks, > Florian > -- 宋方睿
Re: [PATCH 3/5] gcc: Add --nostdlib++ option
On Sun, Dec 5, 2021 at 6:43 AM Richard Purdie via Gcc-patches
wrote:
>
> On Thu, 2021-12-02 at 20:04 -0700, Jeff Law wrote:
> >
> > On 10/28/2021 10:39 AM, Richard Purdie wrote:
> > > On Thu, 2021-10-28 at 08:51 -0600, Jeff Law wrote:
> > > > On 10/27/2021 2:05 PM, Richard Purdie via Gcc-patches wrote:
> > > > > OpenEmbedded/Yocto Project builds libgcc and the other gcc runtime
> > > > > libraries
> > > > > separately from the compiler and slightly differently to the standard
> > > > > gcc build.
> > > > >
> > > > > In general this works well but in trying to build them separately we
> > > > > run into
> > > > > an issue since we're using our gcc, not xgcc and there is no way to
> > > > > tell configure
> > > > > to use libgcc but not look for libstdc++.
> > > > >
> > > > > This adds such an option allowing such configurations to work.
> > > > >
> > > > > 2021-10-26 Richard Purdie
> > > > >
> > > > > gcc/c-family/ChangeLog:
> > > > >
> > > > > * c.opt: Add --nostdlib++ option
> > > > >
> > > > > gcc/cp/ChangeLog:
> > > > >
> > > > > * g++spec.c (lang_specific_driver): Add --nostdlib++ option
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > > * doc/invoke.texi: Document --nostdlib++ option
> > > > > * gcc.c: Add --nostdlib++ option
> > > > Couldn't you use -nostdlib then explicitly add -lgcc?
> > > >
> > > > If that works, that would seem better to me compared to adding an option
> > > > to specs processing that is really only useful to one build
> > > > system/procedure.
> > > It sounds great in principle but I've never been able to get it to work.
> > > With
> > > "-nostdinc++ -nostdlib" I miss the startup files so I also tried
> > > "-nostdinc++ -
> > > nodefaultlibs -lgcc". The latter gets further and I can build libstdc++
> > > but the
> > > resulting library doesn't link into applications correctly.
> > Can you be more specific about "doesn't link into applications
> > correctly". I'm still hesitant to add another option if we can
> > reasonably avoid it.
>
> I took a step back and had another look at what our build is doing and why we
> need this. Our build builds the different components separately in many cases
> so
> libstdc++ is built separately since that allows us to tune it to specific
> targets whilst the core gcc is architecture specific.
>
> When we run configure for libstdc++, we have to configure CXX. We can
> configure
> it as:
>
> CXX="${HOST_PREFIX}g++ -nostdinc++"
>
> however that gives errors about a missing libstdc++ during configure tests
> (e.g.
> the atomic ones) since the library isn't present yet and we're trying to build
> it. When I last ran into this I added the nostdlib++ option to mirror the
> stdinc++ one and this solved the problem.
>
> Adding -lgcc doesn't seem to work, binaries using libstdc++ segfault on some
> architectures (mips64 and ppc). I suspect this is a link ordering issue which
> we
> have little control of from the CXX variable but I haven't got deeply into it
> as
> I got the feeling it would be a pain to try and correct, we need the
> compiler's
> automatic linking behaviour which you can't emulate from one commandline.
>
> I did also try -nostdlib and -nodefaultlibs with various libraries added in
> but
> it never seems to work well giving segfaults, again as I suspect the linking
> is
> order specific.
>
> Thinking about the problem from scratch again, I wondered if a dummy libstdc++
> would be enough to make the configure tests work correctly. I've found that I
> can do something like:
>
> mkdir -p XXX/dummylib
> touch XXX/dummylib/libstdc++.so
> export CXX="${CXX} -nostdinc++ -LXXX/dummylib"
>
> and the configure works correctly and the resulting libs don't segfault on
> target.
>
> This is a bit of a hack but probably no worse than some of the other things we
> have to do to build so if you're not keen on the patch we could go with this.
> It
> did seem at least worth discussing our use case though! :)
>
> Cheers,
>
> Richard
>
Drive-by: is -nostdlib++ is implemented, hope its semantics are
similar to clang -nostdlib++ (https://reviews.llvm.org/D35780 since
2017)
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
On Fri, Oct 8, 2021 at 10:57 AM Fāng-ruì Sòng wrote: > > On Fri, Sep 24, 2021 at 11:29 AM H.J. Lu wrote: > > > > On Fri, Sep 24, 2021 at 11:14 AM Fāng-ruì Sòng wrote: > > > > > > On Fri, Sep 24, 2021 at 10:41 AM H.J. Lu wrote: > > > > > > > > On Fri, Sep 24, 2021 at 10:29 AM Fāng-ruì Sòng > > > > wrote: > > > > > > > > > > On Tue, Sep 21, 2021 at 7:08 PM Fāng-ruì Sòng > > > > > wrote: > > > > > > > > > > > > On Tue, Sep 21, 2021 at 6:57 PM H.J. Lu wrote: > > > > > > > > > > > > > > On Tue, Sep 21, 2021 at 9:16 AM Uros Bizjak > > > > > > > wrote: > > > > > > > > > > > > > > > > On Mon, Sep 20, 2021 at 8:20 PM Fāng-ruì Sòng via Gcc-patches > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > PING^5 > > > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > PING^4 > > > > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > > > > > > > > > > > One major design goal of PIE was to avoid copy relocations. > > > > > > > > > > The original patch for GCC 5 caused problems for many years. > > > > > > > > > > > > > > > > > > > > On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng > > > > > > > > > > wrote: > > > > > > > > > >> > > > > > > > > > >> PING^3 > > > > > > > > > >> https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > >> > > > > > > > > > >> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng > > > > > > > > > >> wrote: > > > > > > > > > >> > > > > > > > > > > >> > PING^2 > > > > > > > > > >> > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > >> > > > > > > > > > > >> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng > > > > > > > > > >> > wrote: > > > > > > > > > >> > > > > > > > > > > > >> > > Ping > > > > > > > > > >> > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > >> > > > > > > > > > > > >> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song > > > > > > > > > >> > > wrote: > > > > > > > > > >> > > > > > > > > > > > > >> > > > This was introduced in 2014-12 to use local binding > > > > > > > > > >> > > > for external symbols > > > > > > > > > >> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for > > > > > > > > > >> > > > years which mostly > > > > > > > > > >> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, > > > > > > > > > >> > > > HAVE_LD_PIE_COPYRELOC > > > > > > > > > >> > > > should retire now. > > > > > > > > > >> > > > > > > > > > > > > >> > > > One design goal of -fPIE was to avoid copy > > > > > > > > > >> > > > relocations. > > > > > > > > > >> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. > > > > > > > > > >> > > > With this change, the > > > > > > > > > >> > > > -fPIE behavior of x86-64 will be closer to x86-32 > > > > > > > > > >> > > > and other targets. >
Re: [PATCH] elf: Add __libc_get_static_tls_bounds [BZ #16291]
On Tue, Oct 19, 2021 at 12:37 PM Fāng-ruì Sòng wrote:
>
> On Thu, Oct 14, 2021 at 5:13 PM Fangrui Song wrote:
> >
> > On 2021-10-06, Fangrui Song wrote:
> > >On 2021-09-27, Fangrui Song wrote:
> > >>On 2021-09-27, Florian Weimer wrote:
> > >>>* Fangrui Song:
> > >>>
> > Sanitizer runtimes need static TLS boundaries for a variety of use
> > cases.
> >
> > * asan/hwasan/msan/tsan need to unpoison static TLS blocks to prevent
> > false
> > positives due to reusing the TLS blocks with a previous thread.
> > * lsan needs TCB for pointers into pthread_setspecific regions.
> >
> > See https://maskray.me/blog/2021-02-14-all-about-thread-local-storage
> > for details.
> >
> > compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp GetTls has
> > to infer the static TLS bounds from TP, _dl_get_tls_static_info, and
> > hard-coded TCB sizes. Currently this is somewhat robust for
> > aarch64/powerpc64/x86-64 but is brittle for many other architectures.
> >
> > This patch implements __libc_get_static_tls_bounds@@GLIBC_PRIVATE which
> > is available in Android bionic since API level 31. This API allows the
> > sanitizer code to be more robust. _dl_get_tls_static_info@@GLIBC_PRIVATE
> > can probably be removed when Clang/GCC sanitizers drop reliance on it.
> > I am unclear whether the version should be GLIBC_2.*.
> > >>>
> > >>>Does this really cover the right memory region? I assume LSAN needs
> > >>>something that identifies pointers to malloc'ed memory that are stored
> > >>>in non-malloc'ed (mmap'ed) memory. The static TLS region is certainly a
> > >>>place where such pointers can be stored. But struct pthread also
> > >>>contains other such pointers: the DTV, the TPP data, and POSIX TLS
> > >>>(pthread_setspecific) data, and struct pthread is not obviously part of
> > >>>the static TLS region.
> > >>
> > >>I know the pthread_setspecific leak detection is brittle but it is
> > >>currently implemented this way ;-)
> > >>
> > >>https://maskray.me/blog/2021-02-14-all-about-thread-local-storage says
> > >>
> > >>"On glibc, GetTls returned range includes
> > >>pthread::{specific_1stblock,specific} for thread-specific data keys.
> > >>There is currently a hack to ignore allocations from ld.so allocated
> > >>dynamic TLS blocks. Note: if the pthread::{specific_1stblock,specific}
> > >>pointers are encrypted, lsan cannot track the allocation."
> > >>
> > >>If pthread::{specific_1stblock,specific} use an XOR technique (like
> > >>__cxa_atexit/setjmp) the pthread_setspecific leak detection will stop
> > >>working :(
> > >>
> > >>---
> > >>
> > >>In any case, the pthread_setspecific leak detection is a relatively
> > >>minor issue. The big issue is asan/msan/tsan false positives due to
> > >>reusing an (exited) thread stack or its TLS blocks.
> > >>
> > >>Around
> > >>https://code.woboq.org/llvm/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp.html#435
> > >>there is very long messy code hard coding the thread descriptor size in
> > >>glibc.
> > >>
> > >>Android `__libc_get_static_tls_bounds(_addr, _addr);` is the
> > >>most robust one.
> > >>
> > >>---
> > >>
> > >>I ported sanitizers to musl (https://reviews.llvm.org/D93848)
> > >>in LLVM 12.0.0 and fixed some TLS block detection aarch64/ppc64 issues
> > >>(https://reviews.llvm.org/D98926 and its follow-up, due to the
> > >>complexity I couldn't get it right in the first place), so I have some
> > >>understanding about sanitizers' TLS usage.
> > >
> > >Adhemerval showed me that the __libc_get_static_tls_bounds behavior is
> > >expected on aarch64 as well (
> > >__libc_get_static_tls_bounds should match sanitizer GetTls)
> > >
> > >From https://gist.github.com/MaskRay/e035b85dce008f0c6d4997b98354d355
> > >```
> > >$ ./testrun.sh ./test-tls-boundary
> > >+++GetTls: 0x7f9c5fd6c000 4416
> > >get_tls=0x7f9c600b4050
> > >_dl_get_tls_static_info: 4416 64
> > >get_static=0x7f9c600b4070
> > >__libc_get_static_tls_bounds: 0x7f9c5fd6c000 4416
> > >```
> > >
> > >
> > >
> > >Is there any concern adding the interface?
> >
> > Gentle ping...
>
>
> CC gcc-patches which ports compiler-rt and may be interested in more
> reliable sanitizers.
PING^3
Re: [PATCH] elf: Add __libc_get_static_tls_bounds [BZ #16291]
On Thu, Oct 14, 2021 at 5:13 PM Fangrui Song wrote:
>
> On 2021-10-06, Fangrui Song wrote:
> >On 2021-09-27, Fangrui Song wrote:
> >>On 2021-09-27, Florian Weimer wrote:
> >>>* Fangrui Song:
> >>>
> Sanitizer runtimes need static TLS boundaries for a variety of use cases.
>
> * asan/hwasan/msan/tsan need to unpoison static TLS blocks to prevent
> false
> positives due to reusing the TLS blocks with a previous thread.
> * lsan needs TCB for pointers into pthread_setspecific regions.
>
> See https://maskray.me/blog/2021-02-14-all-about-thread-local-storage
> for details.
>
> compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp GetTls has
> to infer the static TLS bounds from TP, _dl_get_tls_static_info, and
> hard-coded TCB sizes. Currently this is somewhat robust for
> aarch64/powerpc64/x86-64 but is brittle for many other architectures.
>
> This patch implements __libc_get_static_tls_bounds@@GLIBC_PRIVATE which
> is available in Android bionic since API level 31. This API allows the
> sanitizer code to be more robust. _dl_get_tls_static_info@@GLIBC_PRIVATE
> can probably be removed when Clang/GCC sanitizers drop reliance on it.
> I am unclear whether the version should be GLIBC_2.*.
> >>>
> >>>Does this really cover the right memory region? I assume LSAN needs
> >>>something that identifies pointers to malloc'ed memory that are stored
> >>>in non-malloc'ed (mmap'ed) memory. The static TLS region is certainly a
> >>>place where such pointers can be stored. But struct pthread also
> >>>contains other such pointers: the DTV, the TPP data, and POSIX TLS
> >>>(pthread_setspecific) data, and struct pthread is not obviously part of
> >>>the static TLS region.
> >>
> >>I know the pthread_setspecific leak detection is brittle but it is
> >>currently implemented this way ;-)
> >>
> >>https://maskray.me/blog/2021-02-14-all-about-thread-local-storage says
> >>
> >>"On glibc, GetTls returned range includes
> >>pthread::{specific_1stblock,specific} for thread-specific data keys.
> >>There is currently a hack to ignore allocations from ld.so allocated
> >>dynamic TLS blocks. Note: if the pthread::{specific_1stblock,specific}
> >>pointers are encrypted, lsan cannot track the allocation."
> >>
> >>If pthread::{specific_1stblock,specific} use an XOR technique (like
> >>__cxa_atexit/setjmp) the pthread_setspecific leak detection will stop
> >>working :(
> >>
> >>---
> >>
> >>In any case, the pthread_setspecific leak detection is a relatively
> >>minor issue. The big issue is asan/msan/tsan false positives due to
> >>reusing an (exited) thread stack or its TLS blocks.
> >>
> >>Around
> >>https://code.woboq.org/llvm/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp.html#435
> >>there is very long messy code hard coding the thread descriptor size in
> >>glibc.
> >>
> >>Android `__libc_get_static_tls_bounds(_addr, _addr);` is the
> >>most robust one.
> >>
> >>---
> >>
> >>I ported sanitizers to musl (https://reviews.llvm.org/D93848)
> >>in LLVM 12.0.0 and fixed some TLS block detection aarch64/ppc64 issues
> >>(https://reviews.llvm.org/D98926 and its follow-up, due to the
> >>complexity I couldn't get it right in the first place), so I have some
> >>understanding about sanitizers' TLS usage.
> >
> >Adhemerval showed me that the __libc_get_static_tls_bounds behavior is
> >expected on aarch64 as well (
> >__libc_get_static_tls_bounds should match sanitizer GetTls)
> >
> >From https://gist.github.com/MaskRay/e035b85dce008f0c6d4997b98354d355
> >```
> >$ ./testrun.sh ./test-tls-boundary
> >+++GetTls: 0x7f9c5fd6c000 4416
> >get_tls=0x7f9c600b4050
> >_dl_get_tls_static_info: 4416 64
> >get_static=0x7f9c600b4070
> >__libc_get_static_tls_bounds: 0x7f9c5fd6c000 4416
> >```
> >
> >
> >
> >Is there any concern adding the interface?
>
> Gentle ping...
CC gcc-patches which ports compiler-rt and may be interested in more
reliable sanitizers.
Re: [PATCH] include/longlong.h: Remove incorrect lvalue to rvalue conversion from asm output constraints
On 2021-10-12, Jakub Jelinek wrote:
On Tue, Oct 12, 2021 at 09:21:21AM -0700, Fāng-ruì Sòng wrote:
> > An output constraint takes a lvalue. While GCC happily strips the
> > incorrect lvalue to rvalue conversion, Clang rejects the code by default:
> >
> > error: invalid use of a cast in a inline asm context requiring an
lvalue: remove the cast or build with -fheinous-gnu-extensions
> >
> > The file appears to share the same origin with gmplib longlong.h but
> > they differ much now (gmplib version is much longer).
> >
> > I don't have write access to the git repo.
> > ---
> > include/longlong.h | 186 ++---
> > 1 file changed, 93 insertions(+), 93 deletions(-)
> >
> > diff --git a/include/longlong.h b/include/longlong.h
> > index c3e92e54ecc..0a21a441d2d 100644
> > --- a/include/longlong.h
> > +++ b/include/longlong.h
> > @@ -194,8 +194,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype,
UDItype, UDItype);
> > #if defined (__arc__) && W_TYPE_SIZE == 32
> > #define add_ss(sh, sl, ah, al, bh, bl) \
> >__asm__ ("add.f%1, %4, %5\n\tadc %0, %2, %3" \
> > -: "=r" ((USItype) (sh)), \
> > - "=" ((USItype) (sl)) \
> > +: "=r" (sh), \
> > + "=" (sl) \
> > : "%r" ((USItype) (ah)), \
> >"rICal" ((USItype) (bh)), \
> >"%r" ((USItype) (al)), \
>
> This seems to alter the meanining of existing programs if sh and sl do
> not have the expected type.
>
> I think you need to add a compound expression and temporaries of type
> USItype if you want to avoid the cast.
Add folks who may comment on the output constraint behavior when a
lvalue to rvalue conversion like (`(USItype)`) is added.
Allowing the casts in there is intentional, the comment about this
e.g. in GCC's C FE says:
Really, this should not be here. Users should be using a
proper lvalue, dammit. But there's a long history of using casts
in the output operands. In cases like longlong.h, this becomes a
primitive form of typechecking -- if the cast can be removed, then
the output operand had a type of the proper width; otherwise we'll
get an error.
If you try e.g.:
void
foo (void)
{
int i;
long l;
__asm ("" : "=r" ((unsigned) i));
__asm ("" : "=r" ((long) l));
__asm ("" : "=r" ((long long) l));
__asm ("" : "=r" ((int) l));
__asm ("" : "=r" ((long) i));
}
then on e.g. x86-64 the first 3 asms are accepted by GCC, the last two
rejected, because the modes are different there.
So the above change throws away important typechecking. As it is
used in a macro, something different should verify that if the casts are
removed.
Jakub
Thanks for the detailed explanation. I see the type checking effect now.
In many cases the assembler can provide some checks, e.g.
long foo(long a) {
long r;
__asm__ ("movq %1, %0" : "=r"(r) : "r"(a));
return r;
}
I came here from trying to build glibc with Clang where I came across errors
like
In file included from strtof_l.c:44:
./strtod_l.c:1500:26: error: invalid use of a cast in a inline asm context
requiring an lvalue: remove the cast or build with -fheinous-gnu-extensions
udiv_qrnnd (quot, n, n, 0, d);
~~^~~
./longlong.h:518:24: note: expanded from macro 'udiv_qrnnd'
"=d" ((UDItype) (r)) \
~~~^~
In file included from strtof_l.c:44:
./strtod_l.c:1606:21: error: invalid use of a cast in a inline asm context
requiring an lvalue: remove the cast or build with -fheinous-gnu-extensions
add_ss (n1, n0, r - d0, 0, 0, d0);
^
(Seems that building GCC with Clang doesn't run into the issues?)
If the desire to keep the checking is still strong, perhaps I will just stick
with using the -f option for glibc.
Re: [PATCH] include/longlong.h: Remove incorrect lvalue to rvalue conversion from asm output constraints
On Sun, Oct 10, 2021 at 10:03 PM Florian Weimer wrote:
>
> * Fangrui Song:
>
> > An output constraint takes a lvalue. While GCC happily strips the
> > incorrect lvalue to rvalue conversion, Clang rejects the code by default:
> >
> > error: invalid use of a cast in a inline asm context requiring an
> > lvalue: remove the cast or build with -fheinous-gnu-extensions
> >
> > The file appears to share the same origin with gmplib longlong.h but
> > they differ much now (gmplib version is much longer).
> >
> > I don't have write access to the git repo.
> > ---
> > include/longlong.h | 186 ++---
> > 1 file changed, 93 insertions(+), 93 deletions(-)
> >
> > diff --git a/include/longlong.h b/include/longlong.h
> > index c3e92e54ecc..0a21a441d2d 100644
> > --- a/include/longlong.h
> > +++ b/include/longlong.h
> > @@ -194,8 +194,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype,
> > UDItype, UDItype);
> > #if defined (__arc__) && W_TYPE_SIZE == 32
> > #define add_ss(sh, sl, ah, al, bh, bl) \
> >__asm__ ("add.f%1, %4, %5\n\tadc %0, %2, %3" \
> > -: "=r" ((USItype) (sh)), \
> > - "=" ((USItype) (sl)) \
> > +: "=r" (sh), \
> > + "=" (sl) \
> > : "%r" ((USItype) (ah)), \
> >"rICal" ((USItype) (bh)), \
> >"%r" ((USItype) (al)), \
>
> This seems to alter the meanining of existing programs if sh and sl do
> not have the expected type.
>
> I think you need to add a compound expression and temporaries of type
> USItype if you want to avoid the cast.
Add folks who may comment on the output constraint behavior when a
lvalue to rvalue conversion like (`(USItype)`) is added.
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
On Fri, Sep 24, 2021 at 11:29 AM H.J. Lu wrote: > > On Fri, Sep 24, 2021 at 11:14 AM Fāng-ruì Sòng wrote: > > > > On Fri, Sep 24, 2021 at 10:41 AM H.J. Lu wrote: > > > > > > On Fri, Sep 24, 2021 at 10:29 AM Fāng-ruì Sòng wrote: > > > > > > > > On Tue, Sep 21, 2021 at 7:08 PM Fāng-ruì Sòng > > > > wrote: > > > > > > > > > > On Tue, Sep 21, 2021 at 6:57 PM H.J. Lu wrote: > > > > > > > > > > > > On Tue, Sep 21, 2021 at 9:16 AM Uros Bizjak > > > > > > wrote: > > > > > > > > > > > > > > On Mon, Sep 20, 2021 at 8:20 PM Fāng-ruì Sòng via Gcc-patches > > > > > > > wrote: > > > > > > > > > > > > > > > > PING^5 > > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > PING^4 > > > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > > > > > > > > > One major design goal of PIE was to avoid copy relocations. > > > > > > > > > The original patch for GCC 5 caused problems for many years. > > > > > > > > > > > > > > > > > > On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng > > > > > > > > > wrote: > > > > > > > > >> > > > > > > > > >> PING^3 > > > > > > > > >> https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > >> > > > > > > > > >> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng > > > > > > > > >> wrote: > > > > > > > > >> > > > > > > > > > >> > PING^2 > > > > > > > > >> > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > >> > > > > > > > > > >> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng > > > > > > > > >> > wrote: > > > > > > > > >> > > > > > > > > > > >> > > Ping > > > > > > > > >> > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > >> > > > > > > > > > > >> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song > > > > > > > > >> > > wrote: > > > > > > > > >> > > > > > > > > > > > >> > > > This was introduced in 2014-12 to use local binding > > > > > > > > >> > > > for external symbols > > > > > > > > >> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for > > > > > > > > >> > > > years which mostly > > > > > > > > >> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, > > > > > > > > >> > > > HAVE_LD_PIE_COPYRELOC > > > > > > > > >> > > > should retire now. > > > > > > > > >> > > > > > > > > > > > >> > > > One design goal of -fPIE was to avoid copy relocations. > > > > > > > > >> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. > > > > > > > > >> > > > With this change, the > > > > > > > > >> > > > -fPIE behavior of x86-64 will be closer to x86-32 and > > > > > > > > >> > > > other targets. > > > > > > > > >> > > > > > > > > > > > >> > > > --- > > > > > > > > >> > > > > > > > > > > > >> > > > See > > > > > > > > >> > > > https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html > > > > > > > > >> > > > for a list > > > > > >
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
On Fri, Sep 24, 2021 at 10:41 AM H.J. Lu wrote: > > On Fri, Sep 24, 2021 at 10:29 AM Fāng-ruì Sòng wrote: > > > > On Tue, Sep 21, 2021 at 7:08 PM Fāng-ruì Sòng wrote: > > > > > > On Tue, Sep 21, 2021 at 6:57 PM H.J. Lu wrote: > > > > > > > > On Tue, Sep 21, 2021 at 9:16 AM Uros Bizjak wrote: > > > > > > > > > > On Mon, Sep 20, 2021 at 8:20 PM Fāng-ruì Sòng via Gcc-patches > > > > > wrote: > > > > > > > > > > > > PING^5 > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > > > On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng > > > > > > wrote: > > > > > > > > > > > > > > PING^4 > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > > > > > One major design goal of PIE was to avoid copy relocations. > > > > > > > The original patch for GCC 5 caused problems for many years. > > > > > > > > > > > > > > On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng > > > > > > > wrote: > > > > > > >> > > > > > > >> PING^3 > > > > > > >> https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > >> > > > > > > >> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng > > > > > > >> wrote: > > > > > > >> > > > > > > > >> > PING^2 > > > > > > >> > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > >> > > > > > > > >> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng > > > > > > >> > wrote: > > > > > > >> > > > > > > > > >> > > Ping > > > > > > >> > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > >> > > > > > > > > >> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song > > > > > > >> > > wrote: > > > > > > >> > > > > > > > > > >> > > > This was introduced in 2014-12 to use local binding for > > > > > > >> > > > external symbols > > > > > > >> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years > > > > > > >> > > > which mostly > > > > > > >> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, > > > > > > >> > > > HAVE_LD_PIE_COPYRELOC > > > > > > >> > > > should retire now. > > > > > > >> > > > > > > > > > >> > > > One design goal of -fPIE was to avoid copy relocations. > > > > > > >> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. With > > > > > > >> > > > this change, the > > > > > > >> > > > -fPIE behavior of x86-64 will be closer to x86-32 and > > > > > > >> > > > other targets. > > > > > > >> > > > > > > > > > >> > > > --- > > > > > > >> > > > > > > > > > >> > > > See > > > > > > >> > > > https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html > > > > > > >> > > > for a list > > > > > > >> > > > of fixed and unfixed (e.g. gold incompatibility with > > > > > > >> > > > protected > > > > > > >> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) > > > > > > >> > > > issues. > > > > > > >> > > > > > > > > > >> > > > If you prefer a longer write-up, see > > > > > > >> > > > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected > > > > > > >> > > > --- > > > > > > >> > > > gcc/config.in | 6 --- > > > > > > >> > > > gcc/config/
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
On Tue, Sep 21, 2021 at 7:08 PM Fāng-ruì Sòng wrote: > > On Tue, Sep 21, 2021 at 6:57 PM H.J. Lu wrote: > > > > On Tue, Sep 21, 2021 at 9:16 AM Uros Bizjak wrote: > > > > > > On Mon, Sep 20, 2021 at 8:20 PM Fāng-ruì Sòng via Gcc-patches > > > wrote: > > > > > > > > PING^5 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng > > > > wrote: > > > > > > > > > > PING^4 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > > > One major design goal of PIE was to avoid copy relocations. > > > > > The original patch for GCC 5 caused problems for many years. > > > > > > > > > > On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng > > > > > wrote: > > > > >> > > > > >> PING^3 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > >> > > > > >> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng > > > > >> wrote: > > > > >> > > > > > >> > PING^2 > > > > >> > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > >> > > > > > >> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng > > > > >> > wrote: > > > > >> > > > > > > >> > > Ping > > > > >> > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > >> > > > > > > >> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song > > > > >> > > wrote: > > > > >> > > > > > > > >> > > > This was introduced in 2014-12 to use local binding for > > > > >> > > > external symbols > > > > >> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years > > > > >> > > > which mostly > > > > >> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, > > > > >> > > > HAVE_LD_PIE_COPYRELOC > > > > >> > > > should retire now. > > > > >> > > > > > > > >> > > > One design goal of -fPIE was to avoid copy relocations. > > > > >> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. With this > > > > >> > > > change, the > > > > >> > > > -fPIE behavior of x86-64 will be closer to x86-32 and other > > > > >> > > > targets. > > > > >> > > > > > > > >> > > > --- > > > > >> > > > > > > > >> > > > See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html > > > > >> > > > for a list > > > > >> > > > of fixed and unfixed (e.g. gold incompatibility with protected > > > > >> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues. > > > > >> > > > > > > > >> > > > If you prefer a longer write-up, see > > > > >> > > > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected > > > > >> > > > --- > > > > >> > > > gcc/config.in | 6 --- > > > > >> > > > gcc/config/i386/i386.c| 11 +--- > > > > >> > > > gcc/configure | 52 > > > > >> > > > --- > > > > >> > > > gcc/configure.ac | 48 > > > > >> > > > - > > > > >> > > > gcc/doc/sourcebuild.texi | 3 -- > > > > >> > > > .../gcc.target/i386/pie-copyrelocs-1.c| 14 - > > > > >> > > > .../gcc.target/i386/pie-copyrelocs-2.c| 14 - > > > > >> > > > .../gcc.target/i386/pie-copyrelocs-3.c| 14 - > > > > >> > > > .../gcc.target/i386/pie-copyrelocs-4.c| 17 -- > > > > >> > > > gcc/testsuite/lib/target-supports.exp | 47 > > > > >> > > > - > > >
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
On Tue, Sep 21, 2021 at 6:57 PM H.J. Lu wrote: > > On Tue, Sep 21, 2021 at 9:16 AM Uros Bizjak wrote: > > > > On Mon, Sep 20, 2021 at 8:20 PM Fāng-ruì Sòng via Gcc-patches > > wrote: > > > > > > PING^5 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng wrote: > > > > > > > > PING^4 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > > > > > > One major design goal of PIE was to avoid copy relocations. > > > > The original patch for GCC 5 caused problems for many years. > > > > > > > > On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng > > > > wrote: > > > >> > > > >> PING^3 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > >> > > > >> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng > > > >> wrote: > > > >> > > > > >> > PING^2 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > >> > > > > >> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng > > > >> > wrote: > > > >> > > > > > >> > > Ping https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html > > > >> > > > > > >> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song > > > >> > > wrote: > > > >> > > > > > > >> > > > This was introduced in 2014-12 to use local binding for external > > > >> > > > symbols > > > >> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years which > > > >> > > > mostly > > > >> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, > > > >> > > > HAVE_LD_PIE_COPYRELOC > > > >> > > > should retire now. > > > >> > > > > > > >> > > > One design goal of -fPIE was to avoid copy relocations. > > > >> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. With this > > > >> > > > change, the > > > >> > > > -fPIE behavior of x86-64 will be closer to x86-32 and other > > > >> > > > targets. > > > >> > > > > > > >> > > > --- > > > >> > > > > > > >> > > > See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html for > > > >> > > > a list > > > >> > > > of fixed and unfixed (e.g. gold incompatibility with protected > > > >> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues. > > > >> > > > > > > >> > > > If you prefer a longer write-up, see > > > >> > > > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected > > > >> > > > --- > > > >> > > > gcc/config.in | 6 --- > > > >> > > > gcc/config/i386/i386.c| 11 +--- > > > >> > > > gcc/configure | 52 > > > >> > > > --- > > > >> > > > gcc/configure.ac | 48 > > > >> > > > - > > > >> > > > gcc/doc/sourcebuild.texi | 3 -- > > > >> > > > .../gcc.target/i386/pie-copyrelocs-1.c| 14 - > > > >> > > > .../gcc.target/i386/pie-copyrelocs-2.c| 14 - > > > >> > > > .../gcc.target/i386/pie-copyrelocs-3.c| 14 - > > > >> > > > .../gcc.target/i386/pie-copyrelocs-4.c| 17 -- > > > >> > > > gcc/testsuite/lib/target-supports.exp | 47 > > > >> > > > - > > > >> > > > 10 files changed, 2 insertions(+), 224 deletions(-) > > > >> > > > delete mode 100644 > > > >> > > > gcc/testsuite/gcc.target/i386/pie-copyrelocs-1.c > > > >> > > > delete mode 100644 > > > >> > > > gcc/testsuite/gcc.target/i386/pie-copyrelocs-2.c > > > >> > > > delete mode 100644 > > > >> > > > gcc/testsuite/gcc.target/i386/pie-copyrelocs-3.c > > > >> > > > delete mode 100644 > > > >> > > > gcc/testsuite/gcc.target/i386/pie-copyrelocs-4.c > > > > From x86 maintainer's PoV, the implementation is trivially correct, > > but I have no idea about functionality. HJ, can you please review the > > functionality and post your opinion on the patch to move it forward? > > > > Thanks, > > Uros. > > I prefer to leave it alone and apply this: > > https://gcc.gnu.org/pipermail/gcc-patches/2021-August/576736.html > > instead. I am working to add a nodirect_extern_access attribute based > on feedback at LPC 2021. I think -fpie should be fixed as soon as possible. "Add -f[no-]direct-extern-access" says "-fdirect-extern-access is the default." IMHO this is not a good choice for -fpie. As the description of this patch says, one of the design goals of -fpie is to avoid copy relocations. > In executable and shared library, bind symbols with the STV_PROTECTED > visibility locally As I have repeated many times (also Clang's behavior), STV_PROTECTED visibility symbol should be bound locally regardless of -fno-direct-extern-access. I think it is fair to say all of Michael Matz, Alan Modra, and I think adding so many behaviors under -fno-direct-extern-access is over-engineering (well, because I don't think -fno-direct-extern-access can be selected as the default behavior any time soon). https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected#summary
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
PING^5 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
On Sat, Sep 4, 2021 at 12:11 PM Fāng-ruì Sòng wrote:
>
> PING^4 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>
> One major design goal of PIE was to avoid copy relocations.
> The original patch for GCC 5 caused problems for many years.
>
> On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng wrote:
>>
>> PING^3 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>>
>> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng wrote:
>> >
>> > PING^2 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>> >
>> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng wrote:
>> > >
>> > > Ping https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>> > >
>> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song wrote:
>> > > >
>> > > > This was introduced in 2014-12 to use local binding for external
>> > > > symbols
>> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years which mostly
>> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, HAVE_LD_PIE_COPYRELOC
>> > > > should retire now.
>> > > >
>> > > > One design goal of -fPIE was to avoid copy relocations.
>> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. With this change,
>> > > > the
>> > > > -fPIE behavior of x86-64 will be closer to x86-32 and other targets.
>> > > >
>> > > > ---
>> > > >
>> > > > See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html for a list
>> > > > of fixed and unfixed (e.g. gold incompatibility with protected
>> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues.
>> > > >
>> > > > If you prefer a longer write-up, see
>> > > > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected
>> > > > ---
>> > > > gcc/config.in | 6 ---
>> > > > gcc/config/i386/i386.c| 11 +---
>> > > > gcc/configure | 52 ---
>> > > > gcc/configure.ac | 48 -
>> > > > gcc/doc/sourcebuild.texi | 3 --
>> > > > .../gcc.target/i386/pie-copyrelocs-1.c| 14 -
>> > > > .../gcc.target/i386/pie-copyrelocs-2.c| 14 -
>> > > > .../gcc.target/i386/pie-copyrelocs-3.c| 14 -
>> > > > .../gcc.target/i386/pie-copyrelocs-4.c| 17 --
>> > > > gcc/testsuite/lib/target-supports.exp | 47 -
>> > > > 10 files changed, 2 insertions(+), 224 deletions(-)
>> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-1.c
>> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-2.c
>> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-3.c
>> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-4.c
>> > > >
>> > > > diff --git a/gcc/config.in b/gcc/config.in
>> > > > index e54f59ce0c3..a65bf5d4176 100644
>> > > > --- a/gcc/config.in
>> > > > +++ b/gcc/config.in
>> > > > @@ -1659,12 +1659,6 @@
>> > > > #endif
>> > > >
>> > > >
>> > > > -/* Define 0/1 if your linker supports -pie option with copy reloc. */
>> > > > -#ifndef USED_FOR_TARGET
>> > > > -#undef HAVE_LD_PIE_COPYRELOC
>> > > > -#endif
>> > > > -
>> > > > -
>> > > > /* Define if your PowerPC linker has .gnu.attributes long double
>> > > > support. */
>> > > > #ifndef USED_FOR_TARGET
>> > > > #undef HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE
>> > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> > > > index 915f89f571a..5ec3c6fd0c9 100644
>> > > > --- a/gcc/config/i386/i386.c
>> > > > +++ b/gcc/config/i386/i386.c
>> > > > @@ -10579,11 +10579,7 @@ legitimate_pic_address_disp_p (rtx disp)
>> > > > return true;
>> > > > }
>> > > > else if (!SYMBOL_REF_FAR_ADDR_P (op0)
>> > > > - && (SYMBOL_REF_LOCAL_P (op0)
>> > > > - || (HAVE_LD_PIE_COPYRELOC
>> > > > - && flag_pie
>> > > > - && !SYMBOL_REF_WEAK (op0)
>> > > > - && !SYMBOL_REF_FUNCTION_P (op0)))
>> > > > + && SYMBOL_REF_LOCAL_P (op0)
>> > > >&& ix86_cmodel != CM_LARGE_PIC)
>> > > > return true;
>> > > > break;
>> > > > @@ -22892,10 +22888,7 @@ ix86_atomic_assign_expand_fenv (tree *hold,
>> > > > tree *clear, tree *update)
>> > > > static bool
>> > > > ix86_binds_local_p (const_tree exp)
>> > > > {
>> > > > - return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
>> > > > - (!flag_pic
>> > > > - || (TARGET_64BIT
>> > > > - && HAVE_LD_PIE_COPYRELOC !=
>> > > > 0)));
>> > > > + return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
>> > > > !flag_pic);
>> > > > }
>> > > > #endif
>> > > >
>> > > > diff --git a/gcc/configure b/gcc/configure
>> > > > index
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
PING^4 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
One major design goal of PIE was to avoid copy relocations.
The original patch for GCC 5 caused problems for many years.
On Wed, Aug 18, 2021 at 11:54 PM Fāng-ruì Sòng wrote:
> PING^3 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>
> On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng wrote:
> >
> > PING^2 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> >
> > On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng
> wrote:
> > >
> > > Ping https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> > >
> > > On Tue, May 11, 2021 at 8:29 PM Fangrui Song
> wrote:
> > > >
> > > > This was introduced in 2014-12 to use local binding for external
> symbols
> > > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years which
> mostly
> > > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, HAVE_LD_PIE_COPYRELOC
> > > > should retire now.
> > > >
> > > > One design goal of -fPIE was to avoid copy relocations.
> > > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. With this change,
> the
> > > > -fPIE behavior of x86-64 will be closer to x86-32 and other targets.
> > > >
> > > > ---
> > > >
> > > > See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html for a
> list
> > > > of fixed and unfixed (e.g. gold incompatibility with protected
> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues.
> > > >
> > > > If you prefer a longer write-up, see
> > > >
> https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected
> > > > ---
> > > > gcc/config.in | 6 ---
> > > > gcc/config/i386/i386.c| 11 +---
> > > > gcc/configure | 52
> ---
> > > > gcc/configure.ac | 48
> -
> > > > gcc/doc/sourcebuild.texi | 3 --
> > > > .../gcc.target/i386/pie-copyrelocs-1.c| 14 -
> > > > .../gcc.target/i386/pie-copyrelocs-2.c| 14 -
> > > > .../gcc.target/i386/pie-copyrelocs-3.c| 14 -
> > > > .../gcc.target/i386/pie-copyrelocs-4.c| 17 --
> > > > gcc/testsuite/lib/target-supports.exp | 47 -
> > > > 10 files changed, 2 insertions(+), 224 deletions(-)
> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-1.c
> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-2.c
> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-3.c
> > > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-4.c
> > > >
> > > > diff --git a/gcc/config.in b/gcc/config.in
> > > > index e54f59ce0c3..a65bf5d4176 100644
> > > > --- a/gcc/config.in
> > > > +++ b/gcc/config.in
> > > > @@ -1659,12 +1659,6 @@
> > > > #endif
> > > >
> > > >
> > > > -/* Define 0/1 if your linker supports -pie option with copy reloc.
> */
> > > > -#ifndef USED_FOR_TARGET
> > > > -#undef HAVE_LD_PIE_COPYRELOC
> > > > -#endif
> > > > -
> > > > -
> > > > /* Define if your PowerPC linker has .gnu.attributes long double
> support. */
> > > > #ifndef USED_FOR_TARGET
> > > > #undef HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE
> > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > > > index 915f89f571a..5ec3c6fd0c9 100644
> > > > --- a/gcc/config/i386/i386.c
> > > > +++ b/gcc/config/i386/i386.c
> > > > @@ -10579,11 +10579,7 @@ legitimate_pic_address_disp_p (rtx disp)
> > > > return true;
> > > > }
> > > > else if (!SYMBOL_REF_FAR_ADDR_P (op0)
> > > > - && (SYMBOL_REF_LOCAL_P (op0)
> > > > - || (HAVE_LD_PIE_COPYRELOC
> > > > - && flag_pie
> > > > - && !SYMBOL_REF_WEAK (op0)
> > > > - && !SYMBOL_REF_FUNCTION_P (op0)))
> > > > + && SYMBOL_REF_LOCAL_P (op0)
> > > >&& ix86_cmodel != CM_LARGE_PIC)
> > > > return true;
> > > > break;
> > > > @@ -22892,10 +22888,7 @@ ix86_atomic_assign_expand_fenv (tree *hold,
> tree *clear, tree *update)
> > > > static bool
> > > > ix86_binds_local_p (const_tree exp)
> > > > {
> > > > - return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> > > > - (!flag_pic
> > > > - || (TARGET_64BIT
> > > > - && HAVE_LD_PIE_COPYRELOC !=
> 0)));
> > > > + return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> !flag_pic);
> > > > }
> > > > #endif
> > > >
> > > > diff --git a/gcc/configure b/gcc/configure
> > > > index f03fe888384..c500f5ca11e 100755
> > > > --- a/gcc/configure
> > > > +++ b/gcc/configure
> > > > @@ -29968,58 +29968,6 @@ fi
> > > > { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_ld_pie" >&5
> > > > $as_echo "$gcc_cv_ld_pie" >&6; }
> > > >
> > > > -{ $as_echo
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
PING^3 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
On Fri, Jun 4, 2021 at 3:04 PM Fāng-ruì Sòng wrote:
>
> PING^2 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>
> On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng wrote:
> >
> > Ping https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
> >
> > On Tue, May 11, 2021 at 8:29 PM Fangrui Song wrote:
> > >
> > > This was introduced in 2014-12 to use local binding for external symbols
> > > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years which mostly
> > > nullify the benefit of HAVE_LD_PIE_COPYRELOC, HAVE_LD_PIE_COPYRELOC
> > > should retire now.
> > >
> > > One design goal of -fPIE was to avoid copy relocations.
> > > HAVE_LD_PIE_COPYRELOC has deviated from the goal. With this change, the
> > > -fPIE behavior of x86-64 will be closer to x86-32 and other targets.
> > >
> > > ---
> > >
> > > See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html for a list
> > > of fixed and unfixed (e.g. gold incompatibility with protected
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues.
> > >
> > > If you prefer a longer write-up, see
> > > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected
> > > ---
> > > gcc/config.in | 6 ---
> > > gcc/config/i386/i386.c| 11 +---
> > > gcc/configure | 52 ---
> > > gcc/configure.ac | 48 -
> > > gcc/doc/sourcebuild.texi | 3 --
> > > .../gcc.target/i386/pie-copyrelocs-1.c| 14 -
> > > .../gcc.target/i386/pie-copyrelocs-2.c| 14 -
> > > .../gcc.target/i386/pie-copyrelocs-3.c| 14 -
> > > .../gcc.target/i386/pie-copyrelocs-4.c| 17 --
> > > gcc/testsuite/lib/target-supports.exp | 47 -
> > > 10 files changed, 2 insertions(+), 224 deletions(-)
> > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-1.c
> > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-2.c
> > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-3.c
> > > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-4.c
> > >
> > > diff --git a/gcc/config.in b/gcc/config.in
> > > index e54f59ce0c3..a65bf5d4176 100644
> > > --- a/gcc/config.in
> > > +++ b/gcc/config.in
> > > @@ -1659,12 +1659,6 @@
> > > #endif
> > >
> > >
> > > -/* Define 0/1 if your linker supports -pie option with copy reloc. */
> > > -#ifndef USED_FOR_TARGET
> > > -#undef HAVE_LD_PIE_COPYRELOC
> > > -#endif
> > > -
> > > -
> > > /* Define if your PowerPC linker has .gnu.attributes long double
> > > support. */
> > > #ifndef USED_FOR_TARGET
> > > #undef HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE
> > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > > index 915f89f571a..5ec3c6fd0c9 100644
> > > --- a/gcc/config/i386/i386.c
> > > +++ b/gcc/config/i386/i386.c
> > > @@ -10579,11 +10579,7 @@ legitimate_pic_address_disp_p (rtx disp)
> > > return true;
> > > }
> > > else if (!SYMBOL_REF_FAR_ADDR_P (op0)
> > > - && (SYMBOL_REF_LOCAL_P (op0)
> > > - || (HAVE_LD_PIE_COPYRELOC
> > > - && flag_pie
> > > - && !SYMBOL_REF_WEAK (op0)
> > > - && !SYMBOL_REF_FUNCTION_P (op0)))
> > > + && SYMBOL_REF_LOCAL_P (op0)
> > >&& ix86_cmodel != CM_LARGE_PIC)
> > > return true;
> > > break;
> > > @@ -22892,10 +22888,7 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree
> > > *clear, tree *update)
> > > static bool
> > > ix86_binds_local_p (const_tree exp)
> > > {
> > > - return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> > > - (!flag_pic
> > > - || (TARGET_64BIT
> > > - && HAVE_LD_PIE_COPYRELOC != 0)));
> > > + return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> > > !flag_pic);
> > > }
> > > #endif
> > >
> > > diff --git a/gcc/configure b/gcc/configure
> > > index f03fe888384..c500f5ca11e 100755
> > > --- a/gcc/configure
> > > +++ b/gcc/configure
> > > @@ -29968,58 +29968,6 @@ fi
> > > { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_ld_pie" >&5
> > > $as_echo "$gcc_cv_ld_pie" >&6; }
> > >
> > > -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker PIE support
> > > with copy reloc" >&5
> > > -$as_echo_n "checking linker PIE support with copy reloc... " >&6; }
> > > -gcc_cv_ld_pie_copyreloc=no
> > > -if test $gcc_cv_ld_pie = yes ; then
> > > - if test $in_tree_ld = yes ; then
> > > -if test "$gcc_cv_gld_major_version" -eq 2 -a
> > > "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt 2;
> > > then
> > > -
Re: [PATCH v3 1/2] Add -f[no-]direct-extern-access
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index cff26909292..7dee311051d 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -10312,13 +10312,17 @@ darwin_local_data_pic (rtx disp)
> }
>
> /* True if the function symbol operand X should be loaded from GOT.
> + If CALL_P is true, X is a call operand.
> +
> + NB: -fno-direct-extern-access doesn't force load from GOT for
> + call.
>
> NB: In 32-bit mode, only non-PIC is allowed in inline assembly
> statements, since a PIC register could not be available at the
> call site. */
>
> bool
> -ix86_force_load_from_GOT_p (rtx x)
> +ix86_force_load_from_GOT_p (rtx x, bool call_p)
> {
>return ((TARGET_64BIT || (!flag_pic && HAVE_AS_IX86_GOT32X))
> && !TARGET_PECOFF && !TARGET_MACHO
> @@ -10326,11 +10330,12 @@ ix86_force_load_from_GOT_p (rtx x)
> && ix86_cmodel != CM_LARGE
> && ix86_cmodel != CM_LARGE_PIC
> && GET_CODE (x) == SYMBOL_REF
> - && SYMBOL_REF_FUNCTION_P (x)
> - && (!flag_plt
> - || (SYMBOL_REF_DECL (x)
> - && lookup_attribute ("noplt",
> - DECL_ATTRIBUTES (SYMBOL_REF_DECL
> (x)
> + && ((!call_p && !flag_direct_extern_access)
> + || (SYMBOL_REF_FUNCTION_P (x)
> + && (!flag_plt
> + || (SYMBOL_REF_DECL (x)
> + && lookup_attribute ("noplt",
> + DECL_ATTRIBUTES
> (SYMBOL_REF_DECL (x)))
> && !SYMBOL_REF_LOCAL_P (x));
> }
>
> @@ -10596,7 +10601,8 @@ legitimate_pic_address_disp_p (rtx disp)
> }
> else if (!SYMBOL_REF_FAR_ADDR_P (op0)
>&& (SYMBOL_REF_LOCAL_P (op0)
> - || (HAVE_LD_PIE_COPYRELOC
> + || (flag_direct_extern_access
> + && HAVE_LD_PIE_COPYRELOC
>&& flag_pie
>&& !SYMBOL_REF_WEAK (op0)
>&& !SYMBOL_REF_FUNCTION_P (op0)))
> @@ -13498,7 +13504,7 @@ ix86_print_operand (FILE *file, rtx x, int code)
>
>if (code == 'P')
> {
> - if (ix86_force_load_from_GOT_p (x))
> + if (ix86_force_load_from_GOT_p (x, true))
> {
> /* For inline assembly statement, load function address
> from GOT with 'P' operand modifier to avoid PLT. */
> @@ -21935,10 +21941,10 @@ int
> asm_preferred_eh_data_format (int code, int global)
> {
>/* PE-COFF is effectively always -fPIC because of the .reloc section. */
> - if (flag_pic || TARGET_PECOFF)
> + if (flag_pic || TARGET_PECOFF || !flag_direct_extern_access)
> {
>int type = DW_EH_PE_sdata8;
> - if (!TARGET_64BIT
> + if (ptr_mode == SImode
> || ix86_cmodel == CM_SMALL_PIC
> || (ix86_cmodel == CM_MEDIUM_PIC && (global || code)))
> type = DW_EH_PE_sdata4;
> @@ -23028,10 +23034,21 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree
> *clear, tree *update)
> static bool
> ix86_binds_local_p (const_tree exp)
> {
> - return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> - (!flag_pic
> - || (TARGET_64BIT
> - && HAVE_LD_PIE_COPYRELOC != 0)));
> + return default_binds_local_p_3 (exp, flag_shlib != 0, true,
> + flag_direct_extern_access,
> + (flag_direct_extern_access
> + && (!flag_pic
> + || (TARGET_64BIT
> + && HAVE_LD_PIE_COPYRELOC != 0;
> +}
Hope we can get rid of HAVE_LD_PIE_COPYRELOC unconditionally, probably
separately.
Patch is here: https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
PING^2 https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
On Mon, May 24, 2021 at 9:43 AM Fāng-ruì Sòng wrote:
>
> Ping https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
>
> On Tue, May 11, 2021 at 8:29 PM Fangrui Song wrote:
> >
> > This was introduced in 2014-12 to use local binding for external symbols
> > for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years which mostly
> > nullify the benefit of HAVE_LD_PIE_COPYRELOC, HAVE_LD_PIE_COPYRELOC
> > should retire now.
> >
> > One design goal of -fPIE was to avoid copy relocations.
> > HAVE_LD_PIE_COPYRELOC has deviated from the goal. With this change, the
> > -fPIE behavior of x86-64 will be closer to x86-32 and other targets.
> >
> > ---
> >
> > See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html for a list
> > of fixed and unfixed (e.g. gold incompatibility with protected
> > https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues.
> >
> > If you prefer a longer write-up, see
> > https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected
> > ---
> > gcc/config.in | 6 ---
> > gcc/config/i386/i386.c| 11 +---
> > gcc/configure | 52 ---
> > gcc/configure.ac | 48 -
> > gcc/doc/sourcebuild.texi | 3 --
> > .../gcc.target/i386/pie-copyrelocs-1.c| 14 -
> > .../gcc.target/i386/pie-copyrelocs-2.c| 14 -
> > .../gcc.target/i386/pie-copyrelocs-3.c| 14 -
> > .../gcc.target/i386/pie-copyrelocs-4.c| 17 --
> > gcc/testsuite/lib/target-supports.exp | 47 -
> > 10 files changed, 2 insertions(+), 224 deletions(-)
> > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-1.c
> > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-2.c
> > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-3.c
> > delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-4.c
> >
> > diff --git a/gcc/config.in b/gcc/config.in
> > index e54f59ce0c3..a65bf5d4176 100644
> > --- a/gcc/config.in
> > +++ b/gcc/config.in
> > @@ -1659,12 +1659,6 @@
> > #endif
> >
> >
> > -/* Define 0/1 if your linker supports -pie option with copy reloc. */
> > -#ifndef USED_FOR_TARGET
> > -#undef HAVE_LD_PIE_COPYRELOC
> > -#endif
> > -
> > -
> > /* Define if your PowerPC linker has .gnu.attributes long double support.
> > */
> > #ifndef USED_FOR_TARGET
> > #undef HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index 915f89f571a..5ec3c6fd0c9 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -10579,11 +10579,7 @@ legitimate_pic_address_disp_p (rtx disp)
> > return true;
> > }
> > else if (!SYMBOL_REF_FAR_ADDR_P (op0)
> > - && (SYMBOL_REF_LOCAL_P (op0)
> > - || (HAVE_LD_PIE_COPYRELOC
> > - && flag_pie
> > - && !SYMBOL_REF_WEAK (op0)
> > - && !SYMBOL_REF_FUNCTION_P (op0)))
> > + && SYMBOL_REF_LOCAL_P (op0)
> >&& ix86_cmodel != CM_LARGE_PIC)
> > return true;
> > break;
> > @@ -22892,10 +22888,7 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree
> > *clear, tree *update)
> > static bool
> > ix86_binds_local_p (const_tree exp)
> > {
> > - return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> > - (!flag_pic
> > - || (TARGET_64BIT
> > - && HAVE_LD_PIE_COPYRELOC != 0)));
> > + return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> > !flag_pic);
> > }
> > #endif
> >
> > diff --git a/gcc/configure b/gcc/configure
> > index f03fe888384..c500f5ca11e 100755
> > --- a/gcc/configure
> > +++ b/gcc/configure
> > @@ -29968,58 +29968,6 @@ fi
> > { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_ld_pie" >&5
> > $as_echo "$gcc_cv_ld_pie" >&6; }
> >
> > -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker PIE support with
> > copy reloc" >&5
> > -$as_echo_n "checking linker PIE support with copy reloc... " >&6; }
> > -gcc_cv_ld_pie_copyreloc=no
> > -if test $gcc_cv_ld_pie = yes ; then
> > - if test $in_tree_ld = yes ; then
> > -if test "$gcc_cv_gld_major_version" -eq 2 -a
> > "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt 2;
> > then
> > - gcc_cv_ld_pie_copyreloc=yes
> > -fi
> > - elif test x$gcc_cv_as != x -a x$gcc_cv_ld != x ; then
> > -# Check if linker supports -pie option with copy reloc
> > -case "$target" in
> > -i?86-*-linux* | x86_64-*-linux*)
> > - cat > conftest1.s < > - .globl a_glob
> > - .data
> > - .type a_glob, @object
> > - .size
Re: [PATCH] x86-64: Remove HAVE_LD_PIE_COPYRELOC
Ping https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570139.html
On Tue, May 11, 2021 at 8:29 PM Fangrui Song wrote:
>
> This was introduced in 2014-12 to use local binding for external symbols
> for -fPIE. Now that we have H.J. Lu's GOTPCRELX for years which mostly
> nullify the benefit of HAVE_LD_PIE_COPYRELOC, HAVE_LD_PIE_COPYRELOC
> should retire now.
>
> One design goal of -fPIE was to avoid copy relocations.
> HAVE_LD_PIE_COPYRELOC has deviated from the goal. With this change, the
> -fPIE behavior of x86-64 will be closer to x86-32 and other targets.
>
> ---
>
> See https://gcc.gnu.org/legacy-ml/gcc/2019-05/msg00215.html for a list
> of fixed and unfixed (e.g. gold incompatibility with protected
> https://sourceware.org/bugzilla/show_bug.cgi?id=19823) issues.
>
> If you prefer a longer write-up, see
> https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected
> ---
> gcc/config.in | 6 ---
> gcc/config/i386/i386.c| 11 +---
> gcc/configure | 52 ---
> gcc/configure.ac | 48 -
> gcc/doc/sourcebuild.texi | 3 --
> .../gcc.target/i386/pie-copyrelocs-1.c| 14 -
> .../gcc.target/i386/pie-copyrelocs-2.c| 14 -
> .../gcc.target/i386/pie-copyrelocs-3.c| 14 -
> .../gcc.target/i386/pie-copyrelocs-4.c| 17 --
> gcc/testsuite/lib/target-supports.exp | 47 -
> 10 files changed, 2 insertions(+), 224 deletions(-)
> delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-1.c
> delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-2.c
> delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-3.c
> delete mode 100644 gcc/testsuite/gcc.target/i386/pie-copyrelocs-4.c
>
> diff --git a/gcc/config.in b/gcc/config.in
> index e54f59ce0c3..a65bf5d4176 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -1659,12 +1659,6 @@
> #endif
>
>
> -/* Define 0/1 if your linker supports -pie option with copy reloc. */
> -#ifndef USED_FOR_TARGET
> -#undef HAVE_LD_PIE_COPYRELOC
> -#endif
> -
> -
> /* Define if your PowerPC linker has .gnu.attributes long double support. */
> #ifndef USED_FOR_TARGET
> #undef HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 915f89f571a..5ec3c6fd0c9 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -10579,11 +10579,7 @@ legitimate_pic_address_disp_p (rtx disp)
> return true;
> }
> else if (!SYMBOL_REF_FAR_ADDR_P (op0)
> - && (SYMBOL_REF_LOCAL_P (op0)
> - || (HAVE_LD_PIE_COPYRELOC
> - && flag_pie
> - && !SYMBOL_REF_WEAK (op0)
> - && !SYMBOL_REF_FUNCTION_P (op0)))
> + && SYMBOL_REF_LOCAL_P (op0)
>&& ix86_cmodel != CM_LARGE_PIC)
> return true;
> break;
> @@ -22892,10 +22888,7 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree
> *clear, tree *update)
> static bool
> ix86_binds_local_p (const_tree exp)
> {
> - return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> - (!flag_pic
> - || (TARGET_64BIT
> - && HAVE_LD_PIE_COPYRELOC != 0)));
> + return default_binds_local_p_3 (exp, flag_shlib != 0, true, true,
> !flag_pic);
> }
> #endif
>
> diff --git a/gcc/configure b/gcc/configure
> index f03fe888384..c500f5ca11e 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -29968,58 +29968,6 @@ fi
> { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_ld_pie" >&5
> $as_echo "$gcc_cv_ld_pie" >&6; }
>
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker PIE support with
> copy reloc" >&5
> -$as_echo_n "checking linker PIE support with copy reloc... " >&6; }
> -gcc_cv_ld_pie_copyreloc=no
> -if test $gcc_cv_ld_pie = yes ; then
> - if test $in_tree_ld = yes ; then
> -if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version"
> -ge 25 -o "$gcc_cv_gld_major_version" -gt 2; then
> - gcc_cv_ld_pie_copyreloc=yes
> -fi
> - elif test x$gcc_cv_as != x -a x$gcc_cv_ld != x ; then
> -# Check if linker supports -pie option with copy reloc
> -case "$target" in
> -i?86-*-linux* | x86_64-*-linux*)
> - cat > conftest1.s < - .globl a_glob
> - .data
> - .type a_glob, @object
> - .size a_glob, 4
> -a_glob:
> - .long 2
> -EOF
> - cat > conftest2.s < - .text
> - .globl main
> - .type main, @function
> -main:
> - movl%eax, a_glob(%rip)
> - .size main, .-main
> - .globl ptr
> - .section.data.rel,"aw",@progbits
> - .type ptr, @object
> -ptr:
> - .quad
Re: [PATCH v2] Add --ld-path= to specify an arbitrary executable as the linker
On Fri, Dec 4, 2020 at 5:45 AM Martin Liška wrote:
>
> PING
>
> May I please ping the patch, it's waiting here for a review
> for quite some time.
>
> Thanks,
> Martin
Ping. I think Martin LGTMed this patch and was waiting for a
maintainer to merge it
> On 7/23/20 12:17 PM, Martin Liška wrote:
> > On 7/21/20 6:07 AM, Fangrui Song wrote:
> >> If the value does not contain any path component separator (e.g. a
> >> slash), the linker will be searched for using COMPILER_PATH followed by
> >> PATH. Otherwise, it is either an absolute path or a path relative to the
> >> current working directory.
> >>
> >> --ld-path= complements and overrides -fuse-ld={bfd,gold,lld}. If in the
> >> future, we want to make dfferent linker option decisions we can let
> >> -fuse-ld= represent the linker flavor and --ld-path= the linker path.
> >
> > Hello.
> >
> > I have just few nits:
> >
> > === ERROR type #3: trailing operator (1 error(s)) ===
> > gcc/collect2.c:1155:14:ld_file_name =
> >
> >>
> >> PR driver/93645
> >> * common.opt (--ld-path=): Add --ld-path=
> >> * opts.c (common_handle_option): Handle OPT__ld_path_.
> >> * gcc.c (driver_handle_option): Likewise.
> >> * collect2.c (main): Likewise.
> >> * doc/invoke.texi: Document --ld-path=.
> >>
> >> ---
> >> Changes in v2:
> >> * Renamed -fld-path= to --ld-path= (clang 12.0.0 new option).
> >>The option does not affect code generation and is not a language
> >> feature,
> >>-f* is not suitable. Additionally, clang has other similar --*-path=
> >>options, e.g. --cuda-path=.
> >> ---
> >> gcc/collect2.c | 63 +++--
> >> gcc/common.opt | 4 +++
> >> gcc/doc/invoke.texi | 9 +++
> >> gcc/gcc.c | 2 +-
> >> gcc/opts.c | 1 +
> >> 5 files changed, 64 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/gcc/collect2.c b/gcc/collect2.c
> >> index f8a5ce45994..caa1b96ab52 100644
> >> --- a/gcc/collect2.c
> >> +++ b/gcc/collect2.c
> >> @@ -844,6 +844,7 @@ main (int argc, char **argv)
> >> const char **ld1;
> >> bool use_plugin = false;
> >> bool use_collect_ld = false;
> >> + const char *ld_path = NULL;
> >> /* The kinds of symbols we will have to consider when scanning the
> >>outcome of a first pass link. This is ALL to start with, then might
> >> @@ -961,12 +962,21 @@ main (int argc, char **argv)
> >> if (selected_linker == USE_DEFAULT_LD)
> >> selected_linker = USE_PLUGIN_LD;
> >> }
> >> -else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
> >> - selected_linker = USE_BFD_LD;
> >> -else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
> >> - selected_linker = USE_GOLD_LD;
> >> -else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
> >> - selected_linker = USE_LLD_LD;
> >> +else if (strncmp (argv[i], "-fuse-ld=", 9) == 0
> >> + && selected_linker != USE_LD_MAX)
> >> + {
> >> +if (strcmp (argv[i] + 9, "bfd") == 0)
> >> + selected_linker = USE_BFD_LD;
> >> +else if (strcmp (argv[i] + 9, "gold") == 0)
> >> + selected_linker = USE_GOLD_LD;
> >> +else if (strcmp (argv[i] + 9, "lld") == 0)
> >> + selected_linker = USE_LLD_LD;
> >> + }
> >> +else if (strncmp (argv[i], "--ld-path=", 10) == 0)
> >> + {
> >> +ld_path = argv[i] + 10;
> >> +selected_linker = USE_LD_MAX;
> >> + }
> >> else if (strncmp (argv[i], "-o", 2) == 0)
> >> {
> >> /* Parse the output filename if it's given so that we can make
> >> @@ -1117,14 +1127,34 @@ main (int argc, char **argv)
> >> ld_file_name = find_a_file (, collect_ld_suffix, X_OK);
> >> use_collect_ld = ld_file_name != 0;
> >> }
> >> - /* Search the compiler directories for `ld'. We have protection against
> >> - recursive calls in find_a_file. */
> >> - if (ld_file_name == 0)
> >> -ld_file_name = find_a_file (, ld_suffixes[selected_linker],
> >> X_OK);
> >> - /* Search the ordinary system bin directories
> >> - for `ld' (if native linking) or `TARGET-ld' (if cross). */
> >> - if (ld_file_name == 0)
> >> -ld_file_name = find_a_file (, full_ld_suffixes[selected_linker],
> >> X_OK);
> >> + if (selected_linker == USE_LD_MAX)
> >> +{
> >> + /* If --ld-path= does not contain a path component separator,
> >> search for
> >> + the command using cpath, then using path. Otherwise find the linker
> >> + relative to the current working directory. */
> >> + if (lbasename (ld_path) == ld_path)
> >> +{
> >> + ld_file_name = find_a_file (, ld_path, X_OK);
> >> + if (ld_file_name == 0)
> >> +ld_file_name = find_a_file (, ld_path, X_OK);
> >> +}
> >> + else if (file_exists (ld_path))
> >> +{
> >
> > ^^^ these braces are not needed.
> >
> >> + ld_file_name = ld_path;
> >> +}
> >> +}
> >> + else
> >> +{
> >> + /* Search the
Re: [PATCH] Don't make -gsplit-dwarf imply -g
On Thu, May 14, 2020 at 12:16 AM Richard Biener wrote: > > On Wed, May 13, 2020 at 5:50 PM Fangrui Song wrote: > > > > On 2020-05-13, Eric Botcazou wrote: > > >> Did I mention I dislike -fsplit-dwarf? ;) > > > > > >Seconded, this will be confusing for almost all users. Since the option > > >only > > >affects debug info generation, it should be prefixed with 'g' in any case. > > > > Updating the semantics of -gsplit-dwarf is actually my favorite as > > well:) > > > > -gsplit-dwarf is not common. Many uses have separate -g. Let's change it. > > > > Attached the patch. > > OK if there are no objections over the weekend. I guess this change needs > documenting in gcc-11/changes.html (which probably does not exist yet, > will take care of that). > > Thanks, > Richard. > > > > > (I also wish -gdwarf-5 did not imply -g but the ship may have shipped.) Richard, are you still going to make this change? (If you do it, I'll happy to ask folks to move forward with the clang patch https://reviews.llvm.org/D80391 ) I've added a note from the original implementer (Cary Coutant) here: https://gcc.gnu.org/pipermail/gcc/2020-July/233074.html On the clang side, I don't think anyone has expressed that they would be upset by a behavior change. Several folks have expressed that the semantics are complex, though, e.g. https://github.com/ccache/ccache/issues/393
Re: [PATCH v3] Add --ld-path= to specify an arbitrary executable as the linker
On Sun, Jul 26, 2020 at 4:02 AM Andreas Schwab wrote: > > On Jul 25 2020, Fāng-ruì Sòng wrote: > > > On Sat, Jul 25, 2020 at 12:09 AM Andreas Schwab > > wrote: > >> > >> On Jul 24 2020, Fangrui Song via Gcc-patches wrote: > >> > >> > This is to mimick nearly lines. collect2 should filter out options like > >> > -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case > >> > 'f'. --ld-path needs its own `else if`. > >> > >> If you handle --ld-path as -fld-path then you don't need that. > >> > > > > --ld-path is a deliberate design choice. -f* is not suitable > > (https://sourceware.org/pipermail/gcc/2020-July/233089.html ) > > clang has several other --*-path= (e.g. --cuda-path) > > I don't agree with that opinion. --foo and -ffoo are equivalent and > interchangeable. > > Andreas. For options only affecting linking, -f* are minority. -fgnu-tm enables language-level constructs. -flto/-fopenmp/-fprofile-arcs/-fsanitize=*/etc affect code generation. -fuse-ld= is an exception. The other options (the majority) don't start with -f.
Re: [PATCH v3] Add --ld-path= to specify an arbitrary executable as the linker
On Sat, Jul 25, 2020 at 12:09 AM Andreas Schwab wrote: > > On Jul 24 2020, Fangrui Song via Gcc-patches wrote: > > > This is to mimick nearly lines. collect2 should filter out options like > > -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case > > 'f'. --ld-path needs its own `else if`. > > If you handle --ld-path as -fld-path then you don't need that. > --ld-path is a deliberate design choice. -f* is not suitable (https://sourceware.org/pipermail/gcc/2020-July/233089.html ) clang has several other --*-path= (e.g. --cuda-path)
Re: [PATCH] Add -fld-path= to specify an arbitrary executable as the linker
On 2020-07-03, Martin Liška wrote:
On 7/2/20 9:34 PM, Fāng-ruì Sòng wrote:
On 2020-07-01, Fāng-ruì Sòng wrote:
On 2020-07-01, Martin Liška wrote:
On 6/30/20 5:32 PM, Fāng-ruì Sòng wrote:
There is some concern about clang's -fuse-ld=path
http://lists.llvm.org/pipermail/cfe-dev/2020-June/065710.html and use
of COMPILER_PATH vs PATH.
Shall we introduce another option like -fld-path=path (PATH is used,
COMPILER_PATH is not used)?
I would recommend first landing a patch to LLVM and then we can do
a corresponding change to GCC.
Martin
Thank a lot for you welcoming words! This is what I intend to add for clang:
https://reviews.llvm.org/D83015
I'll create a GCC patch superseding this one later.
Attached the new patch.
Thank you for the update patch:
From e7f86cdcaf03e4ddb98d0df9d07894d9ffb7d91a Mon Sep 17 00:00:00 2001
From: Fangrui Song
Date: Thu, 2 Jul 2020 12:26:09 -0700
Subject: [PATCH] Add -fld-path= to specify an arbitrary executable as the
linker
The value can be either a relative path (relative to a COMPILER_PATH
directory or a PATH directory) or an absolute path. -fld-path=
complements -fuse-ld={bfd,gold,lld} which specifies the linker flavor.
PR driver/93645
* common.opt (-fld-path=): Add -fld-path=
* opts.c (common_handle_option): Handle OPT_fld_path_.
* gcc.c (driver_handle_option): Likewise.
* collect2.c (main): Likewise.
* doc/invoke.texi: Document -fld-path=.
---
gcc/collect2.c | 57 -
gcc/common.opt | 4
gcc/doc/invoke.texi | 6 +
gcc/gcc.c | 2 +-
gcc/opts.c | 1 +
5 files changed, 53 insertions(+), 17 deletions(-)
diff --git a/gcc/collect2.c b/gcc/collect2.c
index f8a5ce45994..efa652f7f82 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -844,6 +844,7 @@ main (int argc, char **argv)
const char **ld1;
bool use_plugin = false;
bool use_collect_ld = false;
+ const char *ld_path = NULL;
/* The kinds of symbols we will have to consider when scanning the
outcome of a first pass link. This is ALL to start with, then might
@@ -961,12 +962,21 @@ main (int argc, char **argv)
if (selected_linker == USE_DEFAULT_LD)
selected_linker = USE_PLUGIN_LD;
}
- else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
- selected_linker = USE_BFD_LD;
- else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
- selected_linker = USE_GOLD_LD;
- else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
- selected_linker = USE_LLD_LD;
+ else if (strncmp (argv[i], "-fuse-ld=bfd", 9) == 0
+&& selected_linker != USE_LD_MAX)
+ {
This does not seem correct to me. You match -fuse-ld=bfd and then
test other option values in the following block.
This is correct but I probably should use:
- strncmp (argv[i], "-fuse-ld=bfd", 9) == 0
+ strncmp (argv[i], "-fuse-ld=", 9) == 0
+ if (strcmp (argv[i] + 9, "bfd") == 0)
+ selected_linker = USE_BFD_LD;
+ else if (strcmp (argv[i] + 9, "gold") == 0)
+ selected_linker = USE_GOLD_LD;
+ else if (strcmp (argv[i] + 9, "lld") == 0)
+ selected_linker = USE_LLD_LD;
+ }
+ else if (strncmp (argv[i], "-fld-path=", 10) == 0)
+ {
+ ld_path = argv[i] + 10;
+ selected_linker = USE_LD_MAX;
+ }
else if (strncmp (argv[i], "-o", 2) == 0)
{
/* Parse the output filename if it's given so that we can make
@@ -1117,14 +1127,27 @@ main (int argc, char **argv)
ld_file_name = find_a_file (, collect_ld_suffix, X_OK);
use_collect_ld = ld_file_name != 0;
}
- /* Search the compiler directories for `ld'. We have protection against
- recursive calls in find_a_file. */
- if (ld_file_name == 0)
-ld_file_name = find_a_file (, ld_suffixes[selected_linker], X_OK);
- /* Search the ordinary system bin directories
- for `ld' (if native linking) or `TARGET-ld' (if cross). */
- if (ld_file_name == 0)
-ld_file_name = find_a_file (, full_ld_suffixes[selected_linker],
X_OK);
+ if (selected_linker == USE_LD_MAX)
+{
+ /* If -fld-path= does not contain a slash, search for the command using
+the PATH environment variable. */
We also support file systems like Windows where the comment about a slash will
be misleading.
You can just mention relative vs. absolute path.
The behavior is modeled after
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
Command Search and Execution
e. Otherwise, the command shall be searched for using the PATH environment
variable as described in XBD Environment Variables :
is performed if "the command name does not contain any characters".
For your suggestion, I think 'word' can mean a relative path as well, along
with 'rel\path' and 'rel/path'.
Should I say
If -fld-path= does not contain a path component separator
[PATCH] Add -fld-path= to specify an arbitrary executable as the linker
On 2020-07-01, Fāng-ruì Sòng wrote:
On 2020-07-01, Martin Liška wrote:
On 6/30/20 5:32 PM, Fāng-ruì Sòng wrote:
There is some concern about clang's -fuse-ld=path
http://lists.llvm.org/pipermail/cfe-dev/2020-June/065710.html and use
of COMPILER_PATH vs PATH.
Shall we introduce another option like -fld-path=path (PATH is used,
COMPILER_PATH is not used)?
I would recommend first landing a patch to LLVM and then we can do
a corresponding change to GCC.
Martin
Thank a lot for you welcoming words! This is what I intend to add for clang:
https://reviews.llvm.org/D83015
I'll create a GCC patch superseding this one later.
Attached the new patch.
>From e7f86cdcaf03e4ddb98d0df9d07894d9ffb7d91a Mon Sep 17 00:00:00 2001
From: Fangrui Song
Date: Thu, 2 Jul 2020 12:26:09 -0700
Subject: [PATCH] Add -fld-path= to specify an arbitrary executable as the
linker
The value can be either a relative path (relative to a COMPILER_PATH
directory or a PATH directory) or an absolute path. -fld-path=
complements -fuse-ld={bfd,gold,lld} which specifies the linker flavor.
PR driver/93645
* common.opt (-fld-path=): Add -fld-path=
* opts.c (common_handle_option): Handle OPT_fld_path_.
* gcc.c (driver_handle_option): Likewise.
* collect2.c (main): Likewise.
* doc/invoke.texi: Document -fld-path=.
---
gcc/collect2.c | 57 -
gcc/common.opt | 4
gcc/doc/invoke.texi | 6 +
gcc/gcc.c | 2 +-
gcc/opts.c | 1 +
5 files changed, 53 insertions(+), 17 deletions(-)
diff --git a/gcc/collect2.c b/gcc/collect2.c
index f8a5ce45994..efa652f7f82 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -844,6 +844,7 @@ main (int argc, char **argv)
const char **ld1;
bool use_plugin = false;
bool use_collect_ld = false;
+ const char *ld_path = NULL;
/* The kinds of symbols we will have to consider when scanning the
outcome of a first pass link. This is ALL to start with, then might
@@ -961,12 +962,21 @@ main (int argc, char **argv)
if (selected_linker == USE_DEFAULT_LD)
selected_linker = USE_PLUGIN_LD;
}
- else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
- selected_linker = USE_BFD_LD;
- else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
- selected_linker = USE_GOLD_LD;
- else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
- selected_linker = USE_LLD_LD;
+ else if (strncmp (argv[i], "-fuse-ld=bfd", 9) == 0
+ && selected_linker != USE_LD_MAX)
+ {
+ if (strcmp (argv[i] + 9, "bfd") == 0)
+ selected_linker = USE_BFD_LD;
+ else if (strcmp (argv[i] + 9, "gold") == 0)
+ selected_linker = USE_GOLD_LD;
+ else if (strcmp (argv[i] + 9, "lld") == 0)
+ selected_linker = USE_LLD_LD;
+ }
+ else if (strncmp (argv[i], "-fld-path=", 10) == 0)
+ {
+ ld_path = argv[i] + 10;
+ selected_linker = USE_LD_MAX;
+ }
else if (strncmp (argv[i], "-o", 2) == 0)
{
/* Parse the output filename if it's given so that we can make
@@ -1117,14 +1127,27 @@ main (int argc, char **argv)
ld_file_name = find_a_file (, collect_ld_suffix, X_OK);
use_collect_ld = ld_file_name != 0;
}
- /* Search the compiler directories for `ld'. We have protection against
- recursive calls in find_a_file. */
- if (ld_file_name == 0)
-ld_file_name = find_a_file (, ld_suffixes[selected_linker], X_OK);
- /* Search the ordinary system bin directories
- for `ld' (if native linking) or `TARGET-ld' (if cross). */
- if (ld_file_name == 0)
-ld_file_name = find_a_file (, full_ld_suffixes[selected_linker], X_OK);
+ if (selected_linker == USE_LD_MAX)
+{
+ /* If -fld-path= does not contain a slash, search for the command using
+ the PATH environment variable. */
+ if (lbasename (ld_path) == ld_path)
+ ld_file_name = find_a_file (, ld_path, X_OK);
+ else if (file_exists (ld_path))
+ ld_file_name = ld_path;
+}
+ else
+{
+ /* Search the compiler directories for `ld'. We have protection against
+ recursive calls in find_a_file. */
+ if (ld_file_name == 0)
+ ld_file_name = find_a_file (, ld_suffixes[selected_linker], X_OK);
+ /* Search the ordinary system bin directories
+ for `ld' (if native linking) or `TARGET-ld' (if cross). */
+ if (ld_file_name == 0)
+ ld_file_name =
+ find_a_file (, full_ld_suffixes[selected_linker], X_OK);
+}
#ifdef REAL_NM_FILE_NAME
nm_file_name = find_a_file (, REAL_NM_FILE_NAME, X_OK);
@@ -1297,9 +1320,11 @@ main (int argc, char **argv)
#endif
}
else if (!use_collect_ld
- && strncmp (arg, "-fuse-ld=", 9) == 0)
+ && (strncmp (arg, "-fuse-ld=", 9) == 0 ||
+ strncmp (arg, "-fld-path=", 10) == 0))
{
- /* Do not pass -fuse-ld={bfd|gold|lld} to the linker. */
+ /* Do not pass -fuse-ld={bfd|gold|lld} or -fld-path= to the
+ linker. */
ld1--;
ld2--;
}
diff --git a/gcc/common.opt b/gcc/common.opt
index df8af365d1b..d5dfbb5c9c6 100644
---
Re: [PATCH v3] Add -fuse-ld= to specify an arbitrary executable as the linker
On 2020-07-01, Martin Liška wrote: On 6/30/20 5:32 PM, Fāng-ruì Sòng wrote: There is some concern about clang's -fuse-ld=path http://lists.llvm.org/pipermail/cfe-dev/2020-June/065710.html and use of COMPILER_PATH vs PATH. Shall we introduce another option like -fld-path=path (PATH is used, COMPILER_PATH is not used)? I would recommend first landing a patch to LLVM and then we can do a corresponding change to GCC. Martin Thank a lot for you welcoming words! This is what I intend to add for clang: https://reviews.llvm.org/D83015 I'll create a GCC patch superseding this one later. On Tue, Jun 30, 2020 at 6:04 AM Martin Liška wrote: PING^1 On 5/29/20 3:10 AM, Fangrui Song wrote: On 2020-05-25, Martin Liška wrote: On 5/22/20 6:42 AM, Fangrui Song wrote: but I can't fix this one because joining two lines will break the 80-column rule. What about this: diff --git a/gcc/collect2.c b/gcc/collect2.c index cc57a20e08b..e5b54b080f7 100644 --- a/gcc/collect2.c +++ b/gcc/collect2.c @@ -1138,8 +1138,8 @@ main (int argc, char **argv) /* Search the ordinary system bin directories for `ld' (if native linking) or `TARGET-ld' (if cross). */ if (ld_file_name == 0) -ld_file_name = - find_a_file (, full_ld_suffixes[selected_linker], X_OK); +ld_file_name + = find_a_file (, full_ld_suffixes[selected_linker], X_OK); } #ifdef REAL_NM_FILE_NAME Apart from that, the patch is fine. Martin Adding people who may be able to approve and commit on my behalf. This formatting issue seems small enough. Hopefully a maintainer can do it for me.
Re: [PATCH v3] Add -fuse-ld= to specify an arbitrary executable as the linker
There is some concern about clang's -fuse-ld=path http://lists.llvm.org/pipermail/cfe-dev/2020-June/065710.html and use of COMPILER_PATH vs PATH. Shall we introduce another option like -fld-path=path (PATH is used, COMPILER_PATH is not used)? On Tue, Jun 30, 2020 at 6:04 AM Martin Liška wrote: > > PING^1 > > On 5/29/20 3:10 AM, Fangrui Song wrote: > > On 2020-05-25, Martin Liška wrote: > >> On 5/22/20 6:42 AM, Fangrui Song wrote: > >>> but I can't fix this one because joining two lines will break the > >>> 80-column rule. > >> > >> What about this: > >> > >> diff --git a/gcc/collect2.c b/gcc/collect2.c > >> index cc57a20e08b..e5b54b080f7 100644 > >> --- a/gcc/collect2.c > >> +++ b/gcc/collect2.c > >> @@ -1138,8 +1138,8 @@ main (int argc, char **argv) > >> /* Search the ordinary system bin directories > >> for `ld' (if native linking) or `TARGET-ld' (if cross). */ > >> if (ld_file_name == 0) > >> -ld_file_name = > >> - find_a_file (, full_ld_suffixes[selected_linker], X_OK); > >> +ld_file_name > >> + = find_a_file (, full_ld_suffixes[selected_linker], X_OK); > >> } > >> #ifdef REAL_NM_FILE_NAME > >> > >> Apart from that, the patch is fine. > >> > >> Martin > > > > Adding people who may be able to approve and commit on my behalf. > > > > This formatting issue seems small enough. Hopefully a maintainer can do > > it for me. > -- 宋方睿
Re: [PATCH][AArch64] PR92424: Fix -fpatchable-function-entry=N,M with BTI
On 2020-01-21, Szabolcs Nagy wrote: On 21/01/2020 11:34, Mark Rutland wrote: Hi Szabolcs, Answers from a linux dev perspective below. thanks. On Mon, Jan 20, 2020 at 10:53:33AM +, Szabolcs Nagy wrote: i have to ask some linux developers which way they prefer: e.g. -fpatchable-function-entry=3,1 is .section __patchable_function_entries .8byte .Lpatch .text .Lpatch: nop func: nop nop ... with bti the code will be emitted as: .Lpatch: nop func: bti c nop nop ... That looks good to me. but e.g. -fpatchable-function-entry=2,0 has two reasonable approaches with bti: (a) func: .Lpatch: bti c nop nop ... (b) func: bti c .Lpatch: nop nop ... I had assumed (b); that means that .Lpatch consistently points to the first NOP. To my mental model, that seems more consistent than (a). However, I can work with either so long as it's consistent. ... As above, my weak preference is (b), but I can work with either. I just need the behaviour to be consistent. Was there a rationale for LLVM choosing (a) rather than (b), e.g. was that also ease of implementation? If there isn't a rationale otherwise, and if LLVM could also do (b), that would be nice from my PoV. How big is "a bit more changes" for GCC? ".Lpatch: nop; nop" is generated by generic code now which a backend can override, but then i have to copy that logic (~30 lines) to add something after the .Lpatch, or i have to change generic code to have a new hook after the .Lpatch:. to provide more argument for (b) and expand on consistency: right now bti is not emitted if a function is not called indirectly (compiler knows this for some local functions) or bti can be turned off by a target attribute per function. so a mix of bti and non-bti functions are possible, so the user patching code has to deal with this (e.g. read the instructions before patching etc) in the M>0, M!=N case bti is emitted in the middle of a patch area (e.g. 3,1 case above), but some functions will not have bti. i can make it so that if patchable function entry is requested then bti is always added, irrespective of any other option or attribute, but that seems to be a big hammer, without such big hammer solution users will have a difficult time to deal with the additional bti. if the patch area is entirely before the function label, then no changes are needed (M=N), if patch area is after the function label (M=0) then using fix (b) would allow simple user code. in other cases user code will be complicated no matter what. so if linux uses M=0 then i think (b) is preferable. hope this helps. The Clang inconvenience is in the other way... (My previous Clang patch series do not implement M>0. https://reviews.llvm.org/D73070 will add support for M>0.) AsmPrinter (assembly printer/object file emitter) does the following: 1. Emit data before the function entry 2. Emit the function entry label and the label for __patchable_function_entries 3. Emit the function body The function body has already been constructed before AsmPrinter. Among late-stage machine code passes, -fpatchable-function-entry=, -mbranch-protection= (AArch64 BTI) and -fcf-protection= (Intel Indirect Branch Tracking) are implemented as passes which can prepend instructions to the function body. To do (b), step 2 needs to be split. The code generator should somehow know the label for __patchable_function_entries is after bti c/endbr32/endbr64. Before forming a decision, I think we should also consider whether M>0 will be possible in the future. Taking M>0 into consideration, is (a) or (b) more consistent? Linux/arch/arm64/Kconfig currently uses -fpatchable-function-entry=2,0 but M>0 could be useful for other architectures (arch/parisc already uses 1,1).
Re: [PATCH][AArch64] PR92424: Fix -fpatchable-function-entry=N,M with BTI
On 2020-01-19, Fāng-ruì Sòng wrote: On 2020-01-16, Richard Sandiford wrote: Szabolcs Nagy writes: this affects the linux kernel and technically a wrong code bug so this fix tries to be backportable (fixing all issues with -fpatchable-function-entry=N,M will likely require new option). Even for the backportable version, I think it would be better not to duplicate so much varasm stuff. Perhaps instead we could: (a) set a cfun->machine flag in aarch64_declare_function_name to say that we've assembled the label (b) override print_patchable_function_entry so that it prints a BTI if this flag is set and the usual other conditions are true As discussed off-list, it'd be good to avoid a second BTI after the nops if possible. print_patchable_function_entry should be able to find the first instruction using get_insns and next_real_insn, then remove it if it turns out to be a BTI. Thanks, Richard It'd be great to have some tests, e.g. 1. -fpatchable-function-entry=0 -mbranch-protection=bti 2. -fpatchable-function-entry=2 -mbranch-protection=bti I have updated clang to emit `.Lfunc_begin0: bti c; nop; nop` for case 2. The __patchable_function_entries entry points to .Lfunc_begin0 (bti c). (The change is not included in the llvm 10.0 branch.) I think we can address all except the SHF_WRITE issue in a backward compatible manner. For some items listed in https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html , the fixes require GNU as (https://sourceware.org/bugzilla/show_bug.cgi?id=25380 https://sourceware.org/bugzilla/show_bug.cgi?id=25381) and ld features (https://sourceware.org/ml/binutils/2019-11/msg00266.html). Hope someone can take a look at https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00271.html
Re: [PATCH][AArch64] PR92424: Fix -fpatchable-function-entry=N,M with BTI
On 2020-01-16, Richard Sandiford wrote:
Szabolcs Nagy writes:
this affects the linux kernel and technically a wrong code bug
so this fix tries to be backportable (fixing all issues with
-fpatchable-function-entry=N,M will likely require new option).
Even for the backportable version, I think it would be better
not to duplicate so much varasm stuff.
Perhaps instead we could:
(a) set a cfun->machine flag in aarch64_declare_function_name
to say that we've assembled the label
(b) override print_patchable_function_entry so that it prints a BTI if
this flag is set and the usual other conditions are true
As discussed off-list, it'd be good to avoid a second BTI after the
nops if possible. print_patchable_function_entry should be able
to find the first instruction using get_insns and next_real_insn,
then remove it if it turns out to be a BTI.
Thanks,
Richard
It'd be great to have some tests, e.g.
1. -fpatchable-function-entry=0 -mbranch-protection=bti
2. -fpatchable-function-entry=2 -mbranch-protection=bti
I have updated clang to emit `.Lfunc_begin0: bti c; nop; nop` for case 2.
The __patchable_function_entries entry points to .Lfunc_begin0 (bti c).
(The change is not included in the llvm 10.0 branch.)
I think we can address all except the SHF_WRITE issue in a backward
compatible manner. For some items listed in
https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html , the fixes require GNU as
(https://sourceware.org/bugzilla/show_bug.cgi?id=25380
https://sourceware.org/bugzilla/show_bug.cgi?id=25381)
and ld features (https://sourceware.org/ml/binutils/2019-11/msg00266.html).
gcc/ChangeLog:
2020-01-16 Szabolcs Nagy
PR target/92424
* config/aarch64/aarch64.c (aarch64_declare_function_name): Emit BTI c
if the function label is followed by a patch area.
From ac2a46bab60ecc80a453328b4749a951908c02c5 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy
Date: Wed, 15 Jan 2020 12:23:40 +
Subject: [PATCH] [AArch64] PR92424: Fix -fpatchable-function-entry=N,M with
BTI
This is a minimal workaround that emits another BTI after the function
label if that is followed by a patch area.
So before this commit -fpatchable-function-entry=3,1 with bti generates
.section __patchable_function_entries
.8byte .LPFE
.text
.LPFE:
nop
foo:
nop
nop
bti c // or paciasp
...
and after this commit
.section __patchable_function_entries
.8byte .LPFE
.text
.LPFE:
nop
foo:
bti c
nop
nop
bti c // or paciasp
...
There is a new bti insn in the middle of the patchable area unless M=0
(patch area is after the new bti) or M=N (patch area is before the
label, no new bti).
Note that .cfi_startproc and the asynchronous unwind table entry label
comes after the patch area, but whatever effect that has on the newly
inserted bti c, it already affected the insns in the patch area.
Tested on aarch64-none-linux-gnu.
gcc/ChangeLog:
PR target/92424
* config/aarch64/aarch64.c (aarch64_declare_function_name): Emit BTI c
if the function label is followed by a patch area.
---
gcc/config/aarch64/aarch64.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 66e20becaf2..0394c274330 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -18079,6 +18079,39 @@ aarch64_declare_function_name (FILE *stream, const
char* name,
/* Don't forget the type directive for ELF. */
ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function");
ASM_OUTPUT_LABEL (stream, name);
+
+ if (!aarch64_bti_enabled ()
+ || cgraph_node::get (fndecl)->only_called_directly_p ())
+return;
+
+ /* Copy logic from varasm.c assemble_start_function
+ to handle -fpatchable-function-entry=N,M with BTI. */
+ unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
+ unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
+
+ tree patchable_function_entry_attr
+= lookup_attribute ("patchable_function_entry", DECL_ATTRIBUTES (fndecl));
+ if (patchable_function_entry_attr)
+{
+ tree pp_val = TREE_VALUE (patchable_function_entry_attr);
+ tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
+
+ patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
+ patch_area_entry = 0;
+ if (TREE_CHAIN (pp_val) != NULL_TREE)
+ {
+ tree patchable_function_entry_value2
+ = TREE_VALUE (TREE_CHAIN (pp_val));
+ patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
+ }
+}
+
+ if (patch_area_entry > patch_area_size)
+patch_area_entry = 0;
+
+ /* Emit a BTI c if a patch area comes after the function label. */
+ if (patch_area_size > patch_area_entry)
+asm_fprintf (stream, "\thint\t34 // bti c\n");
}
/* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */
[PATCH] Explicitly mark _S_ti() as default visibility to work around clang -fvisibility-inlines-hidden bug
clang (including trunk and many older versions) incorrectly marks static local
variables (__tag) hidden when -fvisibility-inlines-hidden is used.
% cat b.cc
#include
std::shared_ptr foo(int x) {
return std::make_shared(x);
}
% g++-8 -fvisibility-inlines-hidden -fno-rtti -c b.cc
% readelf -s b.o | grep _S_ti
163: 1 OBJECT UNIQUE DEFAULT 67
_ZZNSt19_Sp_make_shared_tag5_S_tiEvE5__tag
164: 8 FUNCWEAK HIDDEN68
_ZNSt19_Sp_make_shared_tag5_S_tiEv
% ~/Dev/llvm/static-release/bin/clang++ -fvisibility-inlines-hidden -fno-rtti
-c b.cc
% readelf -s b.o | grep _S_ti
129: 16 FUNCWEAK HIDDEN34
_ZNSt19_Sp_make_shared_tag5_S_tiEv
155: 1 OBJECT WEAK HIDDEN 202
_ZZNSt19_Sp_make_shared_tag5_S_tiEvE5__tag
This can lead to multiple instances of __tag when shares objects are used.
The function
virtual void* std::_Sp_counted_ptr_inplace::_M_get_deleter(const
std::type_info& __ti) noexcept
may return nullptr and causes std::make_shared() to return nullptr
(-fvisibility-inlines-hidden -fno-rtti).
After applying this patch (tagging _S_ti() with default visibility to override
-fvisibility-inlines-hidden)
% readelf -s b.o | grep _S_ti
129: 16 FUNCWEAK DEFAULT 34
_ZNSt19_Sp_make_shared_tag5_S_tiEv
155: 1 OBJECT WEAK DEFAULT 202
_ZZNSt19_Sp_make_shared_tag5_S_tiEvE5__tag
This issue caused 10+ check-all tests of a -DUSE_SHARED_LLVM=On build of llvm
(compiled with clang trunk) to SIGSEGV (because std::make_shared returned
nullptr) and this patch fixes it.
* include/bits/shared_ptr_base.h (_S_ti): Use
_GLIBCXX_VISIBILITY(default)
--
宋方睿
>From 6da8cec298766ce043d9c6dcda7b87142228dafb Mon Sep 17 00:00:00 2001
From: Fangrui Song
Date: Thu, 19 Jul 2018 16:40:26 -0700
Subject: [PATCH 1/1] Explicitly mark _S_ti() as default visibility to work
around clang -fvisibility-inlines-hidden bug
clang (including trunk and many older versions) incorrectly marks static
local variables (__tag) hidden when -fvisibility-inlines-hidden is used.
This can lead to multiple instances of __tag when shares objects are used.
* include/bits/shared_ptr_base.h (_S_ti): Use
_GLIBCXX_VISIBILITY(default)
---
libstdc++-v3/include/bits/shared_ptr_base.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h
b/libstdc++-v3/include/bits/shared_ptr_base.h
index f3994da158f..870aeb9bfda 100644
--- a/libstdc++-v3/include/bits/shared_ptr_base.h
+++ b/libstdc++-v3/include/bits/shared_ptr_base.h
@@ -508,7 +508,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
friend class _Sp_counted_ptr_inplace;
static const type_info&
-_S_ti() noexcept
+_S_ti() noexcept _GLIBCXX_VISIBILITY(default)
{
alignas(type_info) static constexpr _Sp_make_shared_tag __tag;
return reinterpret_cast(__tag);
--
2.18.0
