[llvm] r340455 - bpf: fix an assertion in BPFAsmBackend applyFixup()

[Hans Wennborg via llvm-commits]

On Tue, Aug 28, 2018 at 8:08 PM, Tom Stellard <tstellar at redhat.com> wrote:
> On 08/22/2018 02:21 PM, Yonghong Song via llvm-commits wrote:
>> Author: yhs
>> Date: Wed Aug 22 14:21:03 2018
>> New Revision: 340455
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=340455&view=rev
>> Log:
>> bpf: fix an assertion in BPFAsmBackend applyFixup()
>>
>> Fix bug https://bugs.llvm.org/show_bug.cgi?id=38643
>>
>
> Hi Hans,
>
> Would we be able to merge this to the release_70 branch?

Merged in r341038.

Thanks,
Hans

>> In BPFAsmBackend applyFixup(), there is an assertion for FixedValue to be 0.
>> This may not be true, esp. for optimiation level 0.
>> For example, in the above bug, for the following two
>> static variables:
>>   @bpf_map_lookup_elem = internal global i8* (i8*, i8*)*
>>       inttoptr (i64 1 to i8* (i8*, i8*)*), align 8
>>   @bpf_map_update_elem = internal global i32 (i8*, i8*, i8*, i64)*
>>       inttoptr (i64 2 to i32 (i8*, i8*, i8*, i64)*), align 8
>>
>> The static variable @bpf_map_update_elem will have a symbol
>> offset of 8 and a FK_SecRel_8 with FixupValue 8 will cause
>> the assertion if llvm is built with -DLLVM_ENABLE_ASSERTIONS=ON.
>>
>> The above relocations will not exist if the program is compiled
>> with optimization level -O1 and above as the compiler optimizes
>> those static variables away. In the below error message, -O2
>> is suggested as this is the common practice.
>>
>> Note that FixedValue = 0 in applyFixup() does exist and is valid,
>> e.g., for the global variable my_map in the above bug. The bpf
>> loader will process them properly for map_id's before loading
>> the program into the kernel.
>>
>> The static variables, which are not optimized away by compiler,
>> may have FK_SecRel_8 relocation with non-zero FixedValue.
>>
>> The patch removed the offending assertion and will issue
>> a hard error as below if the FixedValue in applyFixup()
>> is not 0.
>>   $ llc -march=bpf -filetype=obj fixup.ll
>>   LLVM ERROR: Unsupported relocation: try to compile with -O2 or above,
>>       or check your static variable usage
>>
>> Signed-off-by: Yonghong Song <yhs at fb.com>
>>
>> Modified:
>>     llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp
>>
>> Modified: llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp?rev=340455&r1=340454&r2=340455&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp (original)
>> +++ llvm/trunk/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp Wed Aug 22 14:21:03 2018
>> @@ -10,6 +10,8 @@
>>  #include "MCTargetDesc/BPFMCTargetDesc.h"
>>  #include "llvm/ADT/StringRef.h"
>>  #include "llvm/MC/MCAsmBackend.h"
>> +#include "llvm/MC/MCAssembler.h"
>> +#include "llvm/MC/MCContext.h"
>>  #include "llvm/MC/MCFixup.h"
>>  #include "llvm/MC/MCObjectWriter.h"
>>  #include "llvm/Support/EndianStream.h"
>> @@ -71,7 +73,12 @@ void BPFAsmBackend::applyFixup(const MCA
>>                                 bool IsResolved,
>>                                 const MCSubtargetInfo *STI) const {
>>    if (Fixup.getKind() == FK_SecRel_4 || Fixup.getKind() == FK_SecRel_8) {
>> -    assert(Value == 0);
>> +    if (Value) {
>> +      MCContext &Ctx = Asm.getContext();
>> +      Ctx.reportError(Fixup.getLoc(),
>> +                      "Unsupported relocation: try to compile with -O2 or above, "
>> +                      "or check your static variable usage");
>> +    }
>>    } else if (Fixup.getKind() == FK_Data_4) {
>>      support::endian::write<uint32_t>(&Data[Fixup.getOffset()], Value, Endian);
>>    } else if (Fixup.getKind() == FK_Data_8) {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>