How much UB is in my compiler?

C and C++ developers have two bug-related headaches: memory leaks and undefined behavior. As you can guess, I'll talk about undefined behavior—and about "my" compiler. To be more precise, I'll talk about the set of compilers and the tools to develop them, the one called LLVM. Why did I say "my" compiler? Our team really likes Clang, a part of LLVM, and regularly use it.

We recently rechecked the LLVM code and even wrote an article about it. This is its sequel: we'll break down the errors that haven't been covered in the previous part.

Foreword

Do you know what undefined behavior (UB) is? You can witness it when a programmer writes the code that the programming language rules let them write, and the code seems good, but the program works incorrectly (or maybe correctly...). UB also means that the standard doesn't guarantee anything on your code.

The program output depends only on the compiler and the target platform. So, we can deduce that compilers can do almost anything with our program, even what we want them to. However, it turns out that the error is just ably hidden, and it's just one's luck that everything is okay.

Undefined behavior can occur for various reasons: the incorrect use of integer types, the incorrect memory handling, data races in parallel execution, and many others. The scariest part is, it reveals only in certain scenarios.

For example, you've already tested the program and deliver it to a client, but, suddenly, nothing works on their side. Although, everything was fine five minutes ago on our side. However, there are no logs or logical explanations here. There's an explanation for this, though. It's all UB fault. However, we need to go through a tough debugging journey to realize it.

Another scenario is possible: we update the compiler, build the program with it, and everything starts crushing. All the tests are red. Oh, gross. All because of a couple code lines that the compiler has begun to use against you.

The code on the agenda provides such an unusual experience that only one thought comes to mind: the developers do know something about UB that mere mortals may not. They seem to "summon" it on purpose.

C++ programmer's guide to Undefined behavior

Meanwhile, we started posting a book about undefined behavior on our website. The author is Dmitry Sviridkin, and the editor is Andrey Karpov. Here's the link to the first part. You may also subscribe to the monthly article digest so as not to miss other parts of the book and new curious content.

Fragment N1

Let's start with an eternal classic in the world of errors—it's a null pointer dereference.

void LineTable::Dump(Stream *s, Target *target, Address::DumpStyle style,
                     Address::DumpStyle fallback_style, bool show_line_ranges) 
{
  const size_t count = m_entries.size();
  LineEntry line_entry;
  SupportFileSP prev_file;   // <=
  for (size_t idx = 0; idx < count; ++idx) {
    ConvertEntryAtIndexToLineEntry(idx, line_entry);
    line_entry.Dump(s, target, *prev_file != *line_entry.original_file_sp, // <=
                    style, fallback_style, show_line_ranges);
    s->EOL();
    prev_file = line_entry.original_file_sp;
  }
}

The analyzer warning:

V522 Dereferencing of the null pointer 'prev_file' might take place. LineTable.cpp 363

As we can see, the analyzer points to the prev_file variable. And this is what the variable (or rather its type) looks like:

typedef std::shared_ptr<lldb_private::SupportFile> SupportFileSP;

When we declare this way, std::shared_ptr is initialized with null. A null pointer dereference leads to undefined behavior.

Let's have a special UB combo meter to count it.

The UB combo meter: 0 —> 1.

Fragment N2

Errors usually occur throughout the code. Want to see how just a single code fragment can plague your code with bugs?

bool Sema::checkStringLiteralArgumentAttr(const AttributeCommonInfo &CI,
                                          const Expr *E, StringRef &Str,
                                          SourceLocation *ArgLocation) 
{
  const auto *Literal = dyn_cast<StringLiteral>(E->IgnoreParenCasts());
  ....
}

As you can see, the E pointer is dereferenced in the first line without check. And then THIS happened.

As proofs, I'll show you where the function is called in the first two warnings.

Here's the first fragment:

static void handleAssumumptionAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
// Handle the case where the attribute has a text message.
StringRef Str;
SourceLocation AttrStrLoc;
if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &AttrStrLoc))
  return;
....
}

Here's the second one:

static void handleWeakRefAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
....
if (AL.getNumArgs() && S.checkStringLiteralArgumentAttr(AL, 0, Str))
....
}

As we can see, 0 is passed as the second parameter in both cases.

"UB can make your cat get pregnant, even if you don't have one." Looking at the number of warnings, can you imagine how many cats could get pregnant?

Do you remember the UB combo meter? So, here's: 1 —> 36.

Fragment N3

Let's take a look at the following structure:

struct ForceCodegenLinking {
    ForceCodegenLinking() {
      // We must reference the passes in such a way that compilers will not
      // delete it all as dead code, even with whole program optimization,
      // yet is effectively a NO-OP. As the compiler isn't smart enough
      // to know that getenv() never returns -1, this will do the job.
      // This is so that globals in the translation units where these functions
      // are defined are forced to be initialized, populating various
      // registries.
      if (std::getenv("bar") != (char*) -1)
        return;

      (void) llvm::createFastRegisterAllocator();
      (void) llvm::createBasicRegisterAllocator();
      (void) llvm::createGreedyRegisterAllocator();
      (void) llvm::createDefaultPBQPRegisterAllocator();

      (void)llvm::createBURRListDAGScheduler(nullptr,
                                             llvm::CodeGenOptLevel::Default);
      (void)llvm::createSourceListDAGScheduler(nullptr,
                                               llvm::CodeGenOptLevel::Default);
      (void)llvm::createHybridListDAGScheduler(nullptr,
                                               llvm::CodeGenOptLevel::Default);
      (void)llvm::createFastDAGScheduler(nullptr,
                                         llvm::CodeGenOptLevel::Default);
      (void)llvm::createDefaultScheduler(nullptr,
                                         llvm::CodeGenOptLevel::Default);
      (void)llvm::createVLIWDAGScheduler(nullptr,
                                         llvm::CodeGenOptLevel::Default);
    }
  } ForceCodegenLinking; // Force link by creating a global definition.
}

In its constructor, a number of functions are called to create some entities. We'll look only at the functions to which arguments are passed (there are six). For example, here are some of them:

ScheduleDAGSDNodes *llvm::createBURRListDAGScheduler(SelectionDAGISel *IS,
                                                    CodeGenOptLevel OptLevel)
{
  const TargetSubtargetInfo &STI = IS->MF->getSubtarget();
  ....
}

Or

ScheduleDAGSDNodes* createDefaultScheduler(SelectionDAGISel *IS,
                                           CodeGenOpt::Level OptLevel) 
{
  const TargetLowering *TLI = IS->TLI;
  const TargetSubtargetInfo &ST = IS->MF->getSubtarget();
  ....
}

If we look at the function call in the constructor, we see that nullptr is passed as its first argument. It's that first IS parameter that's dereferenced in the very first line of the function.

It's strange code. Perhaps the LLVM developers know something and can actually control UB. Or perhaps they just want there to be more cats in our world :D

Every function with the first zero argument gets dereferenced.

So, here are the analyzer warnings:

• V522 Dereferencing of the null pointer might take place. The null pointer is passed into 'createBURRListDAGScheduler' function. Inspect the first argument. Check lines: 'ScheduleDAGRRList.cpp:3147', 'LinkAllCodegenComponents.h:40'.
• V522 Dereferencing of the null pointer might take place. The null pointer is passed into 'createSourceListDAGScheduler' function. Inspect the first argument. Check lines: 'ScheduleDAGRRList.cpp:3161', 'LinkAllCodegenComponents.h:42'.
• V522 Dereferencing of the null pointer might take place. The null pointer is passed into 'createHybridListDAGScheduler' function. Inspect the first argument. Check lines: 'ScheduleDAGRRList.cpp:3175', 'LinkAllCodegenComponents.h:44'.
• ... (the other three are similar)

The UB combo meter: 36 —> 42.

Fragment N4

Another snippet, another UB:

Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned BuiltinID,
                                           const CallExpr *E) 
{
  ....
  unsigned SrcNumElts =
        cast<llvm::FixedVectorType>(Ops[1]->getType())->getNumElements();
  ....
  int Indices[16];
    for (unsigned i = 0; i != DstNumElts; ++i)
      Indices[i] = (i >= SrcNumElts) ? SrcNumElts + (i % SrcNumElts) : i;
  ....
}

The analyzer warning:

V609 Mod by zero. Denominator 'SrcNumElts' == 0. CGBuiltin.cpp:14833

The analyzer suggests us pay attention to SrcNumElts. Let's get to the bottom of this.

We can see that the ternary operator is used in the loop. The check condition: i is greater than or equal to SrcNumElts. When we have SrcNumElts == 0 then SrcNumElts + (i % SrcNumElts) will be executed (there are no checks above, getNumElements can return 0). As we know, the behavior is undefined when we divide by 0 (including division by modulus).

The UB combo meter: 42 —> 43.

Fragment N5

Here's a small function consisting only of the if statements:

static bool StopAtComponentPre(const Symbol &component) {
  if constexpr (componentKind == ComponentKind::Ordered) {
    // Parent components need to be iterated upon after their
    // sub-components in structure constructor analysis.
    return !component.test(Symbol::Flag::ParentComp);
  } else if constexpr (componentKind == ComponentKind::Direct) {
    return true;
  } else if constexpr (componentKind == ComponentKind::Ultimate) {
    return component.has<ProcEntityDetails>() ||
        IsAllocatableOrObjectPointer(&component) ||
        (component.has<ObjectEntityDetails>() &&
            component.get<ObjectEntityDetails>().type() &&
            component.get<ObjectEntityDetails>().type()->AsIntrinsic());
  } else if constexpr (componentKind == ComponentKind::Potential) {
    return !IsPointer(component);
  } else if constexpr (componentKind == ComponentKind::PotentialAndPointer) {
    return true;
  }
}

Here's the analyzer warning:

V591 Non-void function should return a value. tools.cpp:1278

As we can see, the function has no return for the case when all the conditions are false. This can happen because enum class ComponentKind contains another Scope value not provided here. In such a case, the behavior is undefined.

Undefined behavior doesn't necessarily mean that the function will return a random value (true or false). It's exactly whatever you want it to be.

The UB combo meter: 43 —> 44.

Fragment N6

This is the fragment that might seem safe:

bool AppleObjCRuntimeV2::NonPointerISACache::EvaluateNonPointerISA(
    ObjCISA isa, ObjCISA &ret_isa) {
  ....
  if (index > m_indexed_isa_cache.size())
    return false;

  LLDB_LOGF(log, "AOCRT::NPI Evaluate(ret_isa = 0x%" PRIx64 ")",
          (uint64_t)m_indexed_isa_cache[index]);
  ....
}

It doesn't seem bad, there's even a check for the index.

However, there's still an error here. If the stars align so that the index variable is equal to m_indexed_isa_cache.size(), then IT will happen. Yes, we'll get the array overrun and, as a consequence, we'll catch undefined behavior.

The analyzer warning:

V557 Array overrun is possible. The 'index' index is pointing beyond array bound. AppleObjCRuntimeV2.cpp 3308

To fix it, just write like this:

if (index >= m_indexed_isa_cache.size())
    return false;

Here's exactly the same warning but a little lower in the code.

Here's the analyzer warning:

V557 Array overrun is possible. The 'index' index is pointing beyond array bound. AppleObjCRuntimeV2.cpp 3311

The UB combo meter: 44 —> 46.

Fragment N7

As the old saying goes, "all in good time". In the code fragment above, the pointer check is later than its dereference in the constructor initialization list:

lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
    StdlibCoroutineHandleSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
    : SyntheticChildrenFrontEnd(*valobj_sp) {
  if (valobj_sp)
    Update();
}

The analyzer warning:

V664 The 'valobj_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function. Check lines: 99, 100. Coroutines.cpp

The UB combo meter: 44 —> 46.

Fragment N8

In the last code fragment, let's go back to where we started. It's a null dereference but in a different code fragment, though.

void SetInsertPoint(Instruction *I) {
  BB = I->getParent();
  InsertPt = I->getIterator();
  assert(InsertPt != BB->end() && "Can't read debug loc from end()");
  SetCurrentDebugLocation(I->getStableDebugLoc());
}

The analyzer warning:

V522 Dereferencing of the null pointer 'I' might take place. The null pointer is passed into 'SetInsertPoint' function. Inspect the first argument. Check lines: 'IRBuilder.h:188', 'OMPIRBuilder.cpp:5983'.

The analyzer recommends to pay attention to the I parameter. Well, let's do as it suggests and see where the function is called:

if (UnreachableInst *ExitTI =
        dyn_cast<UnreachableInst>(ExitBB->getTerminator())) {
  CurBBTI->eraseFromParent();
  Builder.SetInsertPoint(ExitBB);
} else {
  Builder.SetInsertPoint(ExitTI);
}

It's another strange snippet. If we look at the names of the (ExitTI and ExitBB) pointers and functions that are called via them (getTerminator), we may get the impression that they're intentionally dereferenced using null to crash the program :D Although the result will be undefined.

We get our "terminator" ExitTI from ExitBB->getTerminator(), and first cast it to UnreachableInst. All that occurs in the condition of the if statement. If the terminator is equal to null, the else branch will be executed, where this "cyborg" passed as an argument to the function, and immediately dereferenced. Hasta la vista, baby.

The UB combo meter: 47 —> 48.

Afterword about null dereference

The null dereference topic is as deep as a rabbit hole — or even deeper than we thought. We suggest you have a bit of fun with this fascinating talk about *(char*)0 = 0;

Want to read something curious? Take a look at "Compilation of gripping C++ conference talks from 2023".

Conclusion

The UB combo meter displays 48 and the article is coming to the end.

The undefined behavior examples seemed marvelous. The curious thing is that developers didn't immediately find them while writing the code. Undefined behavior is usually found in less obvious places, and that's why it's even harder to spot.

It's important to bear in mind that developers have to keep an eye out for UB and other bugs all the time. This requires attention and effort. Even experienced developers may encounter undefined behavior and errors, especially when they work with new libraries or difficult, large systems.

That's why I recommend you not ignore compiler and static analyzer warnings and always enhance your testing experience.