teemperor added inline comments.
================ Comment at: lldb/source/Commands/CommandObjectTrace.cpp:119 lldb::TraceSP trace_sp = traceOrErr.get(); - if (m_options.m_verbose) + if (m_options.m_verbose && trace_sp) result.AppendMessageWithFormat("loading trace with plugin %s\n", ---------------- stella.stamenova wrote: > teemperor wrote: > > Do you still have that GCC warning around (or the GCC version that produced > > that warning)? I'm kinda curious what GCC is assuming here. The interface > > never returns a "null" shared_ptr, the shared_ptr either contains a valid > > object or llvm::Error is returned. IIRC the Expected<ptr> return is the > > "new" version of how the Plugin interface should work (the old was just a > > ptr with `nullptr` being an error), so if some GCC version doesn't > > understand the Expected<ptr> pattern then we should maybe have some more > > centralized workaround instead of adding all the unnecessary nullptr checks. > > > > But that shouldn't block this patch, so feel free to land. > The gcc version is: > > `gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0` > > The warning is: > > ``` > /mnt/_work/9/s/llvm-project/lldb/source/Commands/CommandObjectTrace.cpp: In > member function ‘virtual bool > CommandObjectTraceLoad::DoExecute(lldb_private::Args&, > lldb_private::CommandReturnObject&)’: > /mnt/_work/9/s/llvm-project/lldb/source/Commands/CommandObjectTrace.cpp:120:39: > error: ‘%s’ directive argument is null [-Werror=format-overflow=] > 120 | result.AppendMessageWithFormat("loading trace with plugin > %s\n", > | > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 121 | > trace_sp->GetPluginName().AsCString()); > | > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > /mnt/_work/9/s/llvm-project/lldb/source/Commands/CommandObjectTrace.cpp:120:39: > error: ‘%s’ directive argument is null [-Werror=format-overflow=] > ``` > > Interestingly, this is the only place in all of llvm (well, llvm, lld, clang > and lldb) that produces this warning. > Ah, the problem is that we call `AsCString` and because ConstString's API was literally developed in hell this function returns `nullptr` if the ConstString is empty (or the weird null state). I'm gonna make a patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97586/new/ https://reviews.llvm.org/D97586 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits