binlog icon indicating copy to clipboard operation
binlog copied to clipboard

String may be the first string in a section

Open ericharding opened this issue 2 years ago • 10 comments

When searching for the correct section for a string we should consider the case where the string is the very first string in a section.

Without this change sometimes a certain severity level string will be "found" in the wrong section header causing it to be garbage which falls though to the base case in severityFromString and turns into Severity::no_logs or "off" when printing to text.

ericharding avatar Apr 13 '23 15:04 ericharding

Thank! It is a good find. I do not understand though, if it accidentally skips the correct section, why does it find the address in a different section, instead of throwing?

@spektrof this PR looks good to me, please merge. Thanks.

erenon avatar Apr 13 '23 16:04 erenon

Unfortunately, I debugged though this quite a while ago and was just slow posting the merge request. I don't remember why it escaped the loop off the top of my head.

ericharding avatar Apr 13 '23 20:04 ericharding

Hi, The clang-tidy check failed on dependency installation. Can you check it please?

Thanks

spektrof avatar Apr 14 '23 07:04 spektrof

To fix the CI, I backported changes from the main branch here: https://github.com/morganstanley/binlog/pull/165 However, Clang 14 tests fails, and I'm unable to repro with Clang 15. It needs some more work.

erenon avatar Apr 14 '23 09:04 erenon

Seems like it might just be the apt remove command returning a non-zero exit code. I tweaked the ci to match master. I'm not sure if it'll re-run the job automatically of if you have to kick it.

ericharding avatar Apr 14 '23 13:04 ericharding

The issue seen in this ci run is already fixed in main. However, there's a new problem with clang 14 on Linux that affects the hiperf branch. No resolution yet.

erenon avatar Apr 14 '23 13:04 erenon

lld bug filed: https://github.com/llvm/llvm-project/issues/62209

erenon avatar Apr 18 '23 13:04 erenon

So lld uses --no-apply-dynamic-relocs, that means, instead of setting the pointers in the .binlog.esrc section, it creates relocations, that'll be applied during runtime, when the object is loaded. However, the current linux code still reads the section from the disk, not from memory -- I'll need to change that (perhaps by using dl_iterate_phdr).

erenon avatar Apr 18 '23 18:04 erenon

Fixed in and superseded by https://github.com/morganstanley/binlog/pull/165

erenon avatar Apr 20 '23 07:04 erenon

#165 is now merged.

spektrof avatar Apr 24 '23 14:04 spektrof