This is an archive of the discontinued LLVM Phabricator instance.

Fix LLDB build on old Linux kernels (pre-4.1)
ClosedPublic

Authored by calebzulawski on Sep 13 2022, 7:54 AM.

Details

Summary

These fields are guarded elsewhere, but were missing here.

Diff Detail

Event Timeline

calebzulawski created this revision.Sep 13 2022, 7:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 13 2022, 7:54 AM
calebzulawski requested review of this revision.Sep 13 2022, 7:54 AM

Adding reviewers who know the trace feature.

Seems fine at a glance for the functions that return error types already but for example GetEffectiveDataBufferSize check who calls it and might crash if it returns 0.

I'm not exactly sure what llvm_unreachable does, but I don't believe it will return 0 (maybe it aborts?)

llvm_unreachable is equivalent to __builtin_unreachable so yes it aborts with a stack trace. I misread and thought you were returning values still.

Worth trying one of the trace commands in this build and see if lldb crashes or we get a nice "not supported" message. unreachable is fine if we're reasonably sure you won't get there by accident.

In terms of who calls it and might error, it looks like it's all used in IntelPTMultiCoreTrace.cpp

old Linux kernels

Also, could you include the kernel versions (that you know about at least) in the commit message. For the next person who hits this and wonders if your change is what they need to include.

calebzulawski retitled this revision from Fix LLDB build on old Linux kernels to Fix LLDB build on old Linux kernels (pre-4.1).Sep 13 2022, 8:28 AM
wallace accepted this revision.EditedSep 13 2022, 9:14 AM

Lgtm!

This revision is now accepted and ready to land.Sep 13 2022, 9:14 AM

Did this ever land? I only see https://github.com/llvm/llvm-project/commit/d35702efe73010409c75b1f1b64f205cc3b2f6d3.

We have a short time to get this into 15.0.2, so I'd like to land this if it is needed.

I only have newer kernels but replacing the type with what it looks like on an older kernel and undefing PERF_ATTR_SIZE_VER5, lldb does build. So it looks like this is a good thing to land.

This revision was automatically updated to reflect the committed changes.

I've landed this so it has some time to go through bots before a backport to 15.

Apologies that your name isn't in the commit, I did add an author line locally but Arc decided to remove it before landing.

If you do more fixes (and please do) get commit access yourself, or provide a name/email to use as the git author. Harder to miss that way.