[PATCH] D22912: [ELF] - Linkerscript: implemented ASSERT() keyword.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 28 14:35:01 PDT 2016


grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:909
@@ +908,3 @@
+        error(Msg);
+      return Dot;
+    };
----------------
ruiu wrote:
> Is it true that ASSERT() returns the current value of "."?
Hmm I was sure that yes because linkerscript from link I provided contains:

```
/*
	. = ASSERT(_end <= 0x8000, "Setup too big!");
	. = ASSERT(hdr == 0x1f1, "The setup header has the wrong offset!");
	/* Necessary for the very-old-loader check to work... */
	. = ASSERT(__end_init <= 5*512, "init sections too big!");
```

at the end. And I relied on the fact that location counter should not be moved backward as
"location counter may not be moved backwards outside of an output section if so doing creates areas with overlapping LMAs."
(https://sourceware.org/binutils/docs-2.23/ld/Location-Counter.html)

But I just checked gold behavior and it seems it just returns the result of expression instead.
So it allows moving location counter backward to the begin of address space here since assert are placed at the end and
technically no overlapping happens.

I`ll change that then.

================
Comment at: test/ELF/linkerscript/linkerscript-assert.s:4
@@ +3,3 @@
+
+# RUN: echo "SECTIONS { \
+# RUN:  .aaa         : { *(.aaa) } \
----------------
ruiu wrote:
> I don't think we need this complex test case. This can just be something like
> 
>   ASSERT(0, "fail");
> 
> and
> 
>   ASSERT(1, "success");
Ok + I suggest at least to check the return value of ASSERT as it probably can be used and
already had trobles above.


https://reviews.llvm.org/D22912





More information about the llvm-commits mailing list