LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 32249 - PVS-Studio: Use of Uninitialized Variable (CWE-457)
Summary: PVS-Studio: Use of Uninitialized Variable (CWE-457)
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: DebugInfo (show other bugs)
Version: trunk
Hardware: PC Windows NT
: P release blocker
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks: 30996
  Show dependency tree
 
Reported: 2017-03-13 01:31 PDT by Svyatoslav Razmyslov
Modified: 2017-03-16 11:31 PDT (History)
3 users (show)

See Also:
Fixed By Commit(s):


Attachments
possible fix (533 bytes, patch)
2017-03-13 01:31 PDT, Svyatoslav Razmyslov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Svyatoslav Razmyslov 2017-03-13 01:31:54 PDT
Created attachment 18085 [details]
possible fix

We have found a vulnerability (CWE-457) using PVS-Studio tool: PVS-Studio is a static code analyzer for C, C++ and C#: https://www.viva64.com/en/pvs-studio/

Analyzer warning: V573 Uninitialized variable 'BytesToDrop' was used. The variable was used to initialize itself.

static Error mapNameAndUniqueName(....) {
  ....
  size_t BytesLeft = IO.maxFieldLength();
  if (HasUniqueName) {
    .....
    if (BytesNeeded > BytesLeft) {
      size_t BytesToDrop = (BytesNeeded - BytesLeft);
      size_t DropN = std::min(N.size(), BytesToDrop / 2);
      size_t DropU = std::min(U.size(), BytesToDrop - DropN);
      ....
    }
  } else {
    size_t BytesNeeded = Name.size() + 1;
    StringRef N = Name;
    if (BytesNeeded > BytesLeft) {
      size_t BytesToDrop = std::min(N.size(), BytesToDrop); // <=
      N = N.drop_back(BytesToDrop);
    }
    error(IO.mapStringZ(N));
  }
  ....
}
Comment 1 Simon Pilgrim 2017-03-16 09:04:56 PDT
Zachary please can you take a look at this? It was introduced by your patch D26253/rL286304
Comment 2 Zachary Turner 2017-03-16 11:21:25 PDT
Yes, I'll take a look.
Comment 3 Zachary Turner 2017-03-16 11:31:41 PDT
The patch looks fine, but I think I can make it a bit more concise.  We should be able to replace this entire else block with just 

StringRef N(Name);
N = N.take_front(BytesLeft-1);
error(IO.mapStringZ(N));

I looked into why this wasn't caught by msan, and it's because this is a hard-to-encounter edge case that doesn't have test coverage.  I've been improving the testability of this code for a while now, so we should be at the point fairly soon where I can get test coverage for these weird edge cases.

I'm going to close this as fixed and submit the patch shortly.  Thanks Svyatoslav!