Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

variable set in two consecutive try-catch blocks marked as never read [clang-analyzer-deadcode.DeadStores] #32139

Open Quuxplusone opened 7 years ago

Quuxplusone commented 7 years ago
Bugzilla Link PR33167
Status NEW
Importance P normal
Reported by Łukasz Maciejewski (lukasz.m.maciejewski@gmail.com)
Reported on 2017-05-25 07:46:42 -0700
Last modified on 2017-10-09 11:26:37 -0700
Version 3.9
Hardware PC Linux
CC akrzemi1@gmail.com, alexfh@google.com, djasper@google.com, ganna@apple.com, klimek@google.com, llvm-bugs@lists.llvm.org, v.reichelt@netcologne.de, xazax.hun@gmail.com
Fixed by commit(s)
Attachments clang-tidy-error-ex.cpp (282 bytes, text/x-c++src)
Blocks
Blocked by
See also
Created attachment 18508
minimal working example for the bug

Problem found on:
clang-tidy --version
LLVM (http://llvm.org/):
  LLVM version 4.0.0
  Optimized build.
  Default target: x86_64-unknown-linux-gnu
  Host CPU: haswell

A variable set in two consecutive try-catch blocks is marked as never read (I
assume it means it's never read before it is stored to again). However, if the
second function call (bar2 in the example) will throw, the second store will
never happen.

The bug shows for the most basic clang-tidy invocation:
$ clang-tidy clang-tidy-error-ex.cpp
Quuxplusone commented 7 years ago

Attached clang-tidy-error-ex.cpp (282 bytes, text/x-c++src): minimal working example for the bug

Quuxplusone commented 7 years ago
Here's a similar code snippet with just one try-catch block that
produces a false positive with the command

clang-tidy -checks='-*,clang-analyzer-deadcode.DeadStores' bug.cc --

============================
void foo() { throw 0; }

int main()
{
  int i = 0;

  try
  {
    i = 1;
    foo();
    return 0;
  }
  catch(...) {}

  return i;
}
============================

bug.cc:9:5: warning: Value stored to 'i' is never read [clang-analyzer-
deadcode.DeadStores]
    i = 1;
    ^
bug.cc:9:5: note: Value stored to 'i' is never read

Since the program actually returns 1, the store in line 9 cannot be dead.
This happens since clang 3.5.0.
Quuxplusone commented 7 years ago

Same with the following example (in version 3.9):

#include <iostream>

void configure() { throw 0;}

int main()
{
  bool configured = true;

  for (int i = 0; i < 10 ; ++i)
  {
    try {
    configured = false;
    configure();
    configured = true;
    }
    catch (...) {}
  }
  std::cout << configured << std::endl;
}

analyzer complains that store configured = false is never read, but as the output statement shows, we are able to observe the store.

Quuxplusone commented 7 years ago

Is Ted still a good default assignee for the SA bugs?

Quuxplusone commented 6 years ago
(In reply to Alexander Kornienko from comment #3)
> Is Ted still a good default assignee for the SA bugs?

I doubt that, he is no longer the official owner/maintainer of the SA codebase.
I think Anna would be a better default.
Quuxplusone commented 6 years ago

I am CC-ed on all of Ted's bugs. I'll ask Tanya to set the screener to either me or someone else on our team.