r314262 - Emit section information for extern variables.
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 4 13:13:50 PDT 2017
For why, here's the comment from the code I linked to:
/*
* GDB looks for this symbol name when it cannot find PT_DYNAMIC->DT_DEBUG.
* We don't have a PT_DYNAMIC, so it will find this. Now all we have to do
* is arrange for this space to be filled in with the dynamic linker's
* _r_debug contents after they're initialized. That way, attaching GDB to
* this process or examining its core file will find the PIE we loaded, the
* dynamic linker, and all the shared libraries, making debugging pleasant.
*/
On Wed, Oct 4, 2017 at 4:11 PM, Keane, Erich <erich.keane at intel.com> wrote:
> I’ve added Elizabeth, who is the original patch author. Hopefully she can
> help out here. Additionally, Eli did the original review, so hopefully he
> can chime in as well.
>
>
> I believe the necessity for this warning came out of the discussion on
> fixing the section behavior. The issue here is that redeclaring with a
> different ‘section’ can cause some pretty nasty issues, since it isn’t
> terribly clear what happens if the variable is used in the meantime.
>
>
>
> There is 1 change that I can think of that Elizabeth and I discussed,
> which was to only warn in the case where there was a USAGE between these
> two redeclarations. I’m not sure that will allow na_cl to compile, however
> it is my belief that if there IS a usage between link.h:64 and
> nacl_bootstrap.c:434, that this is a bug in nacl that is simply being
> uncovered thanks to this new warning.
>
>
>
> Is there a good/reasonable reason the source of nacl wants to redeclare
> this with a different ‘section’?
>
>
>
>
>
> *From:* thakis at google.com [mailto:thakis at google.com] *On Behalf Of *Nico
> Weber
> *Sent:* Wednesday, October 4, 2017 12:59 PM
> *To:* Keane, Erich <erich.keane at intel.com>
> *Cc:* cfe-commits <cfe-commits at lists.llvm.org>
> *Subject:* Re: r314262 - Emit section information for extern variables.
>
>
>
> Hi Erich,
>
>
>
> this breaks existing code. NaCl does this:
>
>
>
> #include <link.h>
>
> struct r_debug _r_debug __attribute__((nocommon, section(".r_debug")));
>
>
>
> (There's a lengthy-ish comment for why in https://cs.chromium.org/
> chromium/src/native_client/src/trusted/service_runtime/
> linux/nacl_bootstrap.c?q=nacl_bootstrap.c&sq=package:chromium&dr&l=424)
>
>
>
> link.h in e.g. the debian jessie sysroot says:
>
> extern struct r_debug _r_debug;
>
>
>
> After this change, clang complains:
>
>
>
> ../../native_client/src/trusted/service_runtime/linux/nacl_bootstrap.c:434:16:
> error: section attribute is specified on redeclared variable
> [-Werror,-Wsection]
>
> struct r_debug _r_debug __attribute__((nocommon, section(".r_debug")));
>
> ^
>
> ../../build/linux/debian_jessie_amd64-sysroot/usr/include/link.h:67:23:
> note: previous declaration is here
>
> extern struct r_debug _r_debug;
>
>
>
>
>
> This code used to work in clang, and continues to work in gcc. So this
> patch probably isn't quite the right approach. Ideas?
>
>
>
> On Tue, Sep 26, 2017 at 7:42 PM, Erich Keane via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
> Author: erichkeane
> Date: Tue Sep 26 16:42:34 2017
> New Revision: 314262
>
> URL: http://llvm.org/viewvc/llvm-project?rev=314262&view=rev
> Log:
> Emit section information for extern variables.
>
> Currently, if _attribute_((section())) is used for extern variables,
> section information is not emitted in generated IR when the variables are
> used.
> This is expected since sections are not generated for external linkage
> objects.
> However NiosII requires this information as it uses special GP-relative
> accesses
> for any objects that use attribute section (.sdata). GCC keeps this
> attribute in
> middle-end.
>
> This change emits the section information for all targets.
>
> Patch By: Elizabeth Andrews
>
> Differential Revision:https://reviews.llvm.org/D36487
>
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/test/Sema/attr-section.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> DiagnosticSemaKinds.td?rev=314262&r1=314261&r2=314262&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 26
> 16:42:34 2017
> @@ -2620,6 +2620,8 @@ def err_attribute_section_invalid_for_ta
> "argument to 'section' attribute is not valid for this target: %0">;
> def warn_mismatched_section : Warning<
> "section does not match previous declaration">, InGroup<Section>;
> +def warn_attribute_section_on_redeclaration : Warning<
> + "section attribute is specified on redeclared variable">,
> InGroup<Section>;
>
> def err_anonymous_property: Error<
> "anonymous property is not supported">;
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CodeGenModule.cpp?rev=314262&r1=314261&r2=314262&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Sep 26 16:42:34 2017
> @@ -2432,6 +2432,12 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str
> EmitGlobalVarDefinition(D);
> }
>
> + // Emit section information for extern variables.
> + if (D->hasExternalStorage()) {
> + if (const SectionAttr *SA = D->getAttr<SectionAttr>())
> + GV->setSection(SA->getName());
> + }
> +
> // Handle XCore specific ABI requirements.
> if (getTriple().getArch() == llvm::Triple::xcore &&
> D->getLanguageLinkage() == CLanguageLinkage &&
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDecl.cpp?rev=314262&r1=314261&r2=314262&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Sep 26 16:42:34 2017
> @@ -2607,6 +2607,16 @@ void Sema::mergeDeclAttributes(NamedDecl
> }
> }
>
> + // This redeclaration adds a section attribute.
> + if (New->hasAttr<SectionAttr>() && !Old->hasAttr<SectionAttr>()) {
> + if (auto *VD = dyn_cast<VarDecl>(New)) {
> + if (VD->isThisDeclarationADefinition() != VarDecl::Definition) {
> + Diag(New->getLocation(), diag::warn_attribute_section_
> on_redeclaration);
> + Diag(Old->getLocation(), diag::note_previous_declaration);
> + }
> + }
> + }
> +
> if (!Old->hasAttrs())
> return;
>
>
> Modified: cfe/trunk/test/Sema/attr-section.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/
> attr-section.c?rev=314262&r1=314261&r2=314262&view=diff
> ============================================================
> ==================
> --- cfe/trunk/test/Sema/attr-section.c (original)
> +++ cfe/trunk/test/Sema/attr-section.c Tue Sep 26 16:42:34 2017
> @@ -19,3 +19,7 @@ void __attribute__((section("foo,zed")))
> void __attribute__((section("bar,zed"))) test2(void) {} //
> expected-warning {{section does not match previous declaration}}
>
> enum __attribute__((section("NEAR,x"))) e { one }; // expected-error
> {{'section' attribute only applies to functions, methods, properties, and
> global variables}}
> +
> +extern int a; // expected-note {{previous declaration is here}}
> +int *b = &a;
> +extern int a __attribute__((section("foo,zed"))); // expected-warning
> {{section attribute is specified on redeclared variable}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171004/f0171f72/attachment.html>
More information about the cfe-commits
mailing list