lifting-bits / remill

Library for lifting machine code to LLVM bitcode
Apache License 2.0
1.26k stars 142 forks source link

Attributes for memory read/write intrinsics #554

Open tathanhdinh opened 2 years ago

tathanhdinh commented 2 years ago

In lifting some instructions which read/write memory, e.g. mov rax, [rcx]:

define dso_local %struct.Memory* @sub_0(%struct.State* noalias nonnull align 1 %state, i64 %pc, %struct.Memory* noalias %memory) local_unnamed_addr #0 {
  %RCX_ptr = getelementptr inbounds %struct.State, %struct.State* %state, i64 0, i32 6, i32 5, i32 0, i32 0
  %RAX_ptr = getelementptr inbounds %struct.State, %struct.State* %state, i64 0, i32 6, i32 1, i32 0, i32 0
  %PC_ptr = getelementptr inbounds %struct.State, %struct.State* %state, i64 0, i32 6, i32 33, i32 0, i32 0
  %1 = add i64 %pc, 3
  %RCX_ = load i64, i64* %RCX_ptr, align 8
  %2 = tail call i64 @__remill_read_memory_64(%struct.Memory* %memory, i64 %RCX_) #3
  store i64 %2, i64* %RAX_ptr, align 8
  store i64 %1, i64* %PC_ptr, align 8
  %3 = tail call %struct.Memory* @__remill_missing_block(%struct.State* %state, i64 %1, %struct.Memory* %memory)
  ret %struct.Memory* %3
}

; Function Attrs: noduplicate noinline nounwind optnone readnone willreturn
declare dso_local i64 @__remill_read_memory_64(%struct.Memory*, i64) #1

I suppose that __remill_read_memory_64 will be implemented (by some program which uses remill) when needed and inlined, so no special attribute is needed here. But I still wonder, if it would be better if we add some attributes for such an intrinsic (which may help the optimizer!?), e.g.

; Function Attrs: noduplicate noinline nounwind optnone readonly willreturn
declare dso_local i64 @__remill_read_memory_64(%struct.Memory* readonly, i64) #1
pgoodman commented 2 years ago

I suppose that __remill_read_memory_64 will be implemented (by some program which uses remill) when needed and inlined

Yes.

But I still wonder, if it would be better if we add some attributes for such an intrinsic (which may help the optimizer!?)

Ah! Can you see if marking all the Memory * parameters as const will help -- or changing them to const Memory &? Or submit a PR to the IntrinsicTable code that will do this marking.

You should drop by EH slack or ping on on Twitter. We're using Remill to do something cool that may be relevant to your interests :-)

tathanhdinh commented 2 years ago

@pgoodman thank you for the feedback. I've just opened a PR for further discussion (if needed).