google / autofdo

AutoFDO
https://groups.google.com/forum/#!forum/autofdo
Apache License 2.0
526 stars 111 forks source link

[bug] No checking about "requires the first loadable segment to be executable". #171

Open lifengxiang1025 opened 1 year ago

lifengxiang1025 commented 1 year ago

In https://github.com/google/autofdo/commit/54991ff59ee2920c255ed0efa16687e488d7b1f9, remove checking about "requires the first loadable segment to be executable". If first loadable segment isn't RE. It's bad to transfer runtime address to binary address. Why autofdo doesn't use propeller's code about runtime address to binary address?https://github.com/google/autofdo/blob/master/perfdata_reader.cc#L386C2-L395C56

PandaBlueYellow commented 1 year ago

"Why autofdo doesn't use propeller's code about runtime address to binary address?" Where is autofdo's current code for handling this logic?

shenhanc78 commented 1 year ago

"Why autofdo doesn't use propeller's code about runtime address to binary address?"

AutoFDO delegates the mapping work to quipper, whereas propeller writes its own logic to handle this. A little bit history: AutoFDO predates propeller, and when we wrote propeller, we found that having the logic in propeller is the best option, since it is easier and quicker to use, and easier to debug.

Where is autofdo's current code for handling this logic?

perf_data_converter, which is included in AutoFDO as a submodule, is responsible for calculating the offset and filling the DSOAndOffset data structure.

shenhanc78 commented 1 year ago

Also, we are going to do a massive sync this quarter, so this problem will be fixed.

lifengxiang1025 commented 1 year ago

Also, we are going to do a massive sync this quarter, so this problem will be fixed.

Ok.

lifengxiang1025 commented 1 year ago

Where is autofdo's current code for handling this logic?

Quipper give AutoFDO sample's file_offset and AutoFDO simply think this file_offset belongs to first load section. It could be fix like:https://github.com/google/autofdo/blob/master/perfdata_reader.cc#L406C1-L411C4.