[PATCH] D41928: [ELF] Fix SysV hash tables with --no-rosegment

[Shoaib Meenai via llvm-commits]

(reply inline; hopefully Outlook doesn't mess this up too badly)

From: llvm-commits <llvm-commits-bounces at lists.llvm.org> on behalf of Rafael Avila de Espindola via llvm-commits <llvm-commits at lists.llvm.org>
Reply-To: Rafael Avila de Espindola <rafael.espindola at gmail.com>
Date: Wednesday, January 10, 2018 at 7:45 PM
To: "reviews+D41928+public+4e93109c06803cc3 at reviews.llvm.org" <reviews+D41928+public+4e93109c06803cc3 at reviews.llvm.org>, "rafael.espindola+phab at gmail.com" <rafael.espindola+phab at gmail.com>, "grimar at accesssoftek.com" <grimar at accesssoftek.com>, "ruiu at google.com" <ruiu at google.com>
Cc: "llvm-commits at lists.llvm.org" <llvm-commits at lists.llvm.org>
Subject: Re: [PATCH] D41928: [ELF] Fix SysV hash tables with --no-rosegment

Shoaib Meenai via Phabricator <reviews at reviews.llvm.org<mailto:reviews at reviews.llvm.org>> writes:

smeenai created this revision.
smeenai added reviewers: espindola, grimar, ruiu.
Herald added a subscriber: emaste.

When setting up the chain, we copy over the bucket's previous symbol
index, assuming that this index will be 0 (STN_UNDEF) for an unused
bucket (marking the end of the chain). When linking with --no-rosegment,
however, unused buckets will in fact contain the padding value, and so
the hash table will end up containing invalid chains. Explicitly
zero-initialize the buckets to fix this problem. Also explicitly
zero-initialize the chain entry for STN_UNDEF; I don't think this is
actually necessary, but it doesn't hurt, and should make the hash table
identical both with and without --no-rosegment.


Repository:
   rLLD LLVM Linker

https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D41928&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=WNZ-Ei6GsCkrmsPagtyCCpRbriq-8qUz8JSmDjkMvMc&s=qXi5oGQEvi08zDlYOq7eDh0-VCKN7PIb-NxquZyecJg&e=

Files:
   ELF/SyntheticSections.cpp
   test/ELF/sysv-hash-no-rosegment.s


Index: test/ELF/sysv-hash-no-rosegment.s
===================================================================
--- /dev/null
+++ test/ELF/sysv-hash-no-rosegment.s
@@ -0,0 +1,13 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: ld.lld -shared --no-rosegment -o %t %t.o
+# RUN: llvm-readobj -hash-table %t | FileCheck %s
+
+# CHECK:      HashTable {
+# CHECK-NEXT:   Num Buckets: 2
+# CHECK-NEXT:   Num Chains: 2
+# CHECK-NEXT:   Buckets: [1, 0]
+# CHECK-NEXT:   Chains: [0, 0]
+# CHECK-NEXT: }
+
+callq undef at PLT
Index: ELF/SyntheticSections.cpp
===================================================================
--- ELF/SyntheticSections.cpp
+++ ELF/SyntheticSections.cpp
@@ -1843,7 +1843,9 @@
    write32(P++, NumSymbols); // nchain

    uint32_t *Buckets = P;
+  memset(Buckets, STN_UNDEF, NumSymbols * 4);

I would just use 0 in here. Otherwise I get the impression that this
writes 4 *NumSymbols 32 values.

Hmm. Should I use 0 for the other added line (below) as well then? That one is actually dealing with 32-bit quantities, but it seems weird to use 0 in one place and STN_UNDEF in another for what's conceptually supposed to be the same thing.

+  Chains[STN_UNDEF] = STN_UNDEF;

LGTM with that.

Thanks,
Rafael
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=WNZ-Ei6GsCkrmsPagtyCCpRbriq-8qUz8JSmDjkMvMc&s=teF1hX5flwteYpV8dviCoUz7kw7KKltbmyeN6xSn8yk&e=

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180111/ce6c6083/attachment.html>