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

[Tom Stellard via llvm-commits]

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?

-Tom

> 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
>