[PATCH] D87318: [LLD][PowerPC] Add support for R_PPC64_GOT_TLSGD_PCREL34 used in TLS General Dynamic

Mon Sep 14 06:12:42 PDT 2020

stefanp added inline comments.

================
Comment at: lld/ELF/Relocations.cpp:1350
+
+    if (type == R_PPC64_TLSGD && expr == R_TLSDESC_CALL) {
+      if (i == end) {
----------------
MaskRay wrote:
> I am not sure we want to add error checking for every relocation usage. We should add them considering the possibility of such errors (can a compiler/tool/assembly generate such erroneous relocation usage?) 
I do not believe that the compiler can generate either of these situations from C source code. 

I can generate the bad alignment in assembly (by adding `.space 1` for example) but it is not likely that someone will do that.

In general there are a couple of reasons why I add code like this.
First, it is to prevent future development work from creating hidden bugs. Maybe a little like 
https://llvm.org/docs/CodingStandards.html#assert-liberally 
from a philosophical perspective. I know that these are not asserts but I think the argument remains valid...
Second, it is my way of documenting assumptions in the code.

I understand your point of view. You don't want want half the code to be error checking so if you really want me to remove these two errors I can.  I just wanted to give you my perspective on this. Let me know what you think!

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87318/new/

https://reviews.llvm.org/D87318