This is an archive of the discontinued LLVM Phabricator instance.

BPF: fix a CORE optimization bug
ClosedPublic

Authored by yonghong-song on Apr 19 2020, 6:52 PM.

Details

Summary

For the test case in this patch like below

struct t { int a; } __attribute__((preserve_access_index));
int foo(void *); 
int test(struct t *arg) {
    long param[1];
    param[0] = (long)&arg->a;
    return foo(param);
}

The IR right before BPF SimplifyPatchable phase:

%1:gpr = LD_imm64 @"llvm.t:0:0$0:0"
%2:gpr = LDD killed %1:gpr, 0
%3:gpr = ADD_rr %0:gpr(tied-def 0), killed %2:gpr
STD killed %3:gpr, %stack.0.param, 0

After SimplifyPatchable phase, the incorrect IR is generated:

%1:gpr = LD_imm64 @"llvm.t:0:0$0:0"
%3:gpr = ADD_rr %0:gpr(tied-def 0), killed %1:gpr
CORE_MEM killed %3:gpr, 306, %0:gpr, @"llvm.t:0:0$0:0"

Note that CORE_MEM pseudo op is introduced to encode
memory operations related to CORE. In the above, we intend
to check whether we have a store like

*(%3:gpr + 0) = ...

and if this is the case, we could replace it with

*(%0:gpr + @"llvm.t:0:0$0:0"_ = ...

Unfortunately, in the above, IR for the store is

*(%stack.0.param + 0) = %3:gpr

and transformation should not happen.

Note that we won't have problem if the actual CORE
dereference (arg->a) happens.

This patch fixed the problem by skip CORE optimization if
the use of ADD_rr result is not the base address of the store
operation.

Diff Detail

Event Timeline

yonghong-song created this revision.Apr 19 2020, 6:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2020, 6:52 PM
ast accepted this revision.Apr 20 2020, 6:52 PM

lgtm

This revision is now accepted and ready to land.Apr 20 2020, 6:52 PM
This revision was automatically updated to reflect the committed changes.