[PATCH] D30886: [ELF] Pad x86 executable sections with 0xcc int3 instructions

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/OutputSections.cpp:241
   Loc = Buf;
   if (uint32_t Filler = Script<ELFT>::X->getFiller(this->Name))
     fill(Buf, this->Size, Filler);
----------------
I do not think that works perfect. 
That will work wrong if linkerscript sets filler to 0x00 explicitly:
```
.text: { *(.text*) }=0x00
```
In that case it still will fill gaps fith 0xcc, what is not correct.
I had to use llvm::Optional in D30901 for Filler member to fix that.

Though I am not sure how much this case is real, so please 
just check other reviewers opinion about this.


================
Comment at: test/ELF/default-fill.s:5
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld %t.o -o %t.elf
----------------
You also implemented i686 target, so probably need a test for i686-pc-linux triple.


================
Comment at: test/ELF/default-fill.s:10
+# CHECK: cc int3
+# CHECK-NOT: 00 00 addb
+
----------------
I would do check to be more explicit:
```
# RUN: llvm-objdump -s %t.out | FileCheck %s
# CHECK:      11cccccc cccccccc cccccccc cccccccc
# CHECK-NEXT: 22
```

Because what you want to check is just that whole area between input sections
was filled by some pattern.


================
Comment at: test/ELF/default-fill.s:20
+.align 16
+.globl other
+other:
----------------
You do not need .globl _start/.globl other. All you probably need here is 2 sections with some markers at start probably, like:

```

.section        .text.1,"ax"
.align  16
.byte   0x11

.section        .text.2,"ax"
.align  16
.byte   0x22
```


https://reviews.llvm.org/D30886