[clang-analyzer] fix warnings emitted on lldb code base

[apelete]

Summary

The following warnings were reported while running clang analyzer on
LLDB code base:

API: argument with 'nonnull' attribute passed null, on file:

• source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.cpp.

Dead store: dead assignement, on file:

• source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.cpp.

Logic error: branch condition evaluates to a garbage value, on file:

• source/Core/DynamicLoader.cpp

Logic error: called C++ object pointer is null, on files:

• source/API/SBThread.cpp,
• source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp,
• source/DataFormatters/VectorType.cpp (22 warnings in cpp file, suppressed in custom assertion handler in include/lldb/Utility/LLDBAssert.h),
• source/Core/FormatEntity.cpp.

(please note that first revision was sent only to lldb-commits mailing
list, not reviewed yet).

Signed-off-by: Apelete Seketeli <[e-mail]>

[jasonmolenda]

These look like good changes to me, especially the SBThread fix - I'm surprised that hasn't caused a problem. The DynamicLoader change should be unnecessary but I don't have a problem with that. Tamas might want to speak to what he meant to do in DWARFASTParserJava.cpp there. Greg Clayton should probably comment on the FormatEntity change.

source/Core/FormatEntity.cpp

1016

Greg should look at this.

[granata.enrico]

source/Core/FormatEntity.cpp

1016

Personally, this LGTM (the code was originally written by me - Greg moved it around last though)

Greg, feel free to disagree

[jingham]

source/API/SBThread.cpp

926

Can you switch this around and check for log first? We generally don't like to do any work that the log might do if there's no log, and also it makes it easier to see that this whole block is only for logging.

source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp

309–329

Seems to me this whole section of code doesn't do anything reasonable if reg_ctx is null. It would be better to move the reg_ctx check higher up.

[apelete]

[clang-analyzer] fix warnings emitted on lldb code base

Following changes were done in this revision:

• source/API/SBThread.cpp: swith if() statement conditions to branch out early,
• source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp: return early if reg_ctx is null.

[apelete]

Ping :).
Requested changes are done, could someone have a look have this one ?

[clayborg]

These all look fine.

[apelete]

[clang-analyzer] fix warnings emitted on lldb code base

Changes since last revision:

• fast forward rebase on git master branch.

[apelete]

Waiting for review, could someone please have a look at this one ?

[clayborg]

Looks good

[granata.enrico]

I am not an Apple employee working on LLDB anymore

include/lldb/Utility/LLDBAssert.h

 #endif
 
 namespace lldb_private {
     void
     lldb_assert (bool expression,
                  const char* expr_text,
                  const char* func,
                  const char* file,
-                 unsigned int line);
+                 unsigned int line) __attribute__((__noreturn__));
 }
 
 #endif // utility_LLDBAssert_h_

source/API/SBThread.cpp

if (log)
                      frame_desc_strm.GetData());
     }
 
     if (exe_ctx.HasThreadScope())
     {
         bool abort_other_plans = false;
         bool stop_other_threads = false;
         Thread *thread = exe_ctx.GetThreadPtr();
-        if (sb_frame.GetThread().GetThreadID() != thread->GetID())
+        if (sb_frame.GetThread().GetThreadID() != thread->GetID() && log)
         {
             log->Printf("SBThread(%p)::StepOutOfFrame passed a frame from another thread (0x%" PRIx64 " vrs. 0x%" PRIx64 ", returning.",
                         static_cast<void*>(exe_ctx.GetThreadPtr()),
                         sb_frame.GetThread().GetThreadID(),
                         thread->GetID());
         }
 
         ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut (abort_other_plans,

source/Core/DynamicLoader.cpp

else if ((module_sp = target.GetSharedModule(module_spec)))
         UpdateLoadedSections(module_sp, link_map_addr, base_addr, base_addr_is_offset);
     }
     else
     {
         if (base_addr_is_offset)
         {
             // Try to fetch the load address of the file from the process as we need absolute load
             // address to read the file out of the memory instead of a load bias.
-            bool is_loaded;
+            bool is_loaded = false;
             lldb::addr_t load_addr;
             Error error = m_process->GetFileLoadAddress(file, is_loaded, load_addr);
             if (error.Success() && is_loaded)
                 base_addr = load_addr;
         }
 
         if ((module_sp = m_process->ReadModuleFromMemory(file, base_addr)))
         {

source/Core/FormatEntity.cpp

else
             else
             {
                 if (log)
                     log->Printf("[Debugger::FormatPrompt] special_directions for child item: %s",special_directions.data() ? special_directions.data() : "");
             }
 
             if (special_directions.empty())
             {
+                if (item)
                     success &= item->DumpPrintableRepresentation(s,val_obj_display, custom_format);
             }
             else
             {
                 success &= FormatEntity::FormatStringRef(special_directions, s, sc, exe_ctx, nullptr, item, false, false);
             }
 
             if (--max_num_children == 0)
             {

source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp

ABISysV_mips64::SetReturnValueObject(lldb::StackFrameSP &frame_sp, lldb::ValueObjectSP &new_value_sp)
     {
         if (type_flags & eTypeIsInteger ||
             type_flags & eTypeIsPointer )
         {
             lldb::offset_t offset = 0;
             
             if (num_bytes <= 16)
             {
-                const RegisterInfo *r2_info = reg_ctx->GetRegisterInfoByName("r2", 0);
+                const RegisterInfo *r2_info = nullptr;
+
+                if (reg_ctx)
+                  r2_info = reg_ctx->GetRegisterInfoByName("r2", 0);
+
                 if (num_bytes <= 8)
                 {
                     uint64_t raw_value = data.GetMaxU64(&offset, num_bytes);
                         
-                    if (!reg_ctx->WriteRegisterFromUnsigned (r2_info, raw_value))
+                    if (reg_ctx && !reg_ctx->WriteRegisterFromUnsigned (r2_info, raw_value))
                         error.SetErrorString ("failed to write register r2");
                 }
                 else
                 {
                     uint64_t raw_value = data.GetMaxU64(&offset, 8);
-                    if (reg_ctx->WriteRegisterFromUnsigned (r2_info, raw_value))
+                    if (reg_ctx && reg_ctx->WriteRegisterFromUnsigned (r2_info, raw_value))
                     {
                         const RegisterInfo *r3_info = reg_ctx->GetRegisterInfoByName("r3", 0);
                         raw_value = data.GetMaxU64(&offset, num_bytes - offset);
                         
-                        if (!reg_ctx->WriteRegisterFromUnsigned (r3_info, raw_value))
+                        if (reg_ctx && !reg_ctx->WriteRegisterFromUnsigned (r3_info, raw_value))
                             error.SetErrorString ("failed to write register r3");
                     }
                     else
                         error.SetErrorString ("failed to write register r2");
                 }
             }
             else
             {

source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.cpp

for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid(); die = die.GetSibling())
         switch (die.Tag())
         {
             case DW_TAG_member:
             {
                 const char *name = nullptr;
                 DWARFFormValue encoding_uid;
                 uint32_t member_byte_offset = UINT32_MAX;
                 DWARFExpression member_location_expression(dwarf_cu);
-                bool artificial = true;
 
                 DWARFAttributes attributes;
                 size_t num_attributes = die.GetAttributes(attributes);
                 for (size_t i = 0; i < num_attributes; ++i)
                 {
                     DWARFFormValue form_value;
                     if (attributes.ExtractFormValueAtIndex(i, form_value))
                     {
for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid(); die = die.GetSibling())
                                 if (form_value.BlockData())
                                     member_location_expression.CopyOpcodeData(
                                         form_value.BlockData(), form_value.Unsigned(), dwarf_cu->GetByteOrder(),
                                         dwarf_cu->GetAddressByteSize());
                                 else
                                     member_byte_offset = form_value.Unsigned();
                                 break;
                             case DW_AT_artificial:
-                                artificial = form_value.Boolean();
+                                // TODO: Handle when needed
                                 break;
                             case DW_AT_accessibility:
                                 // TODO: Handle when needed
                                 break;
                             default:
                                 assert(false && "Unhandled attribute for DW_TAG_member");
                                 break;
                         }
                     }
                 }
 
-                if (strcmp(name, ".dynamic_type") == 0)
+                if (name && strcmp(name, ".dynamic_type") == 0)
                     m_ast.SetDynamicTypeId(compiler_type, member_location_expression);
                 else
                 {
                     if (Type *member_type = die.ResolveTypeUID(DIERef(encoding_uid)))
                         m_ast.AddMemberToObject(compiler_type, ConstString(name), member_type->GetFullCompilerType(),
                                                 member_byte_offset);
                 }
                 break;