[llvm] r295595 - Add two files lost in rebase, causing build break

Steven Wu via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 20 10:58:25 PST 2017


I committed a fix in r295675 to fix the bot. I change the function prototype to:
std::pair<std::string, std::string> getCounterInfo(unsigned ID) const;
I am not quite sure why I get junk if I print the option name returned as StringRef so I change that to std::string as well. 

Steven

> On Feb 20, 2017, at 10:14 AM, Steven Wu via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Hi Daniel
> 
> This commit is causing ASAN failure due to use after free:
> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/2998/ <http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/2998/>
> 
> 
>> On Feb 18, 2017, at 8:29 PM, Daniel Berlin via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> 
>> Author: dannyb
>> Date: Sat Feb 18 22:29:50 2017
>> New Revision: 295595
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=295595&view=rev <http://llvm.org/viewvc/llvm-project?rev=295595&view=rev>
>> Log:
>> Add two files lost in rebase, causing build break
>> 
>> Added:
>>    llvm/trunk/include/llvm/Support/DebugCounter.h
>>    llvm/trunk/lib/Support/DebugCounter.cpp
>> 
>> Added: llvm/trunk/include/llvm/Support/DebugCounter.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/DebugCounter.h?rev=295595&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/DebugCounter.h?rev=295595&view=auto>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Support/DebugCounter.h (added)
>> +++ llvm/trunk/include/llvm/Support/DebugCounter.h Sat Feb 18 22:29:50 2017
>> @@ -0,0 +1,141 @@
>> +//===- llvm/Support/DebugCounter.h - Debug counter support ------*- C++ -*-===//
>> +//
>> +//                     The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>> +//===----------------------------------------------------------------------===//
>> +// \file This file provides an implementation of debug counters.  Debug counters
>> +// are a tool that let you narrow down a miscompilation to a specific thing
>> +// happening.  To give a use case: Imagine you have a file, very large, and you
>> +// are trying to understand the minimal transformation that breaks it.  Bugpoint
>> +// and bisection is often helpful here in narrowing it down to a specific pass,
>> +// but it's still a very large file, and a very complicated pass to try to
>> +// debug.  That is where debug counting steps in.  You can instrument the pass
>> +// with a debug counter before it does a certain thing, and depending on the
>> +// counts, it will either execute that thing or not.  The debug counter itself
>> +// consists of a skip and a count.  Skip is the number of times shouldExecute
>> +// needs to be called before it returns true.  Count is the number of times to
>> +// return true once Skip is 0.  So a skip=47, count=2 ,would skip the first 47
>> +// executions by returning false from shouldExecute, then execute twice, and
>> +// then return false again.
>> +// Note that a counter set to a negative number will always execute.
>> +
>> +// For a concrete example, during predicateinfo creation, the renaming pass
>> +// replaces each use with a renamed use.
>> +///
>> +// If I use DEBUG_COUNTER to create a counter called "predicateinfo", and
>> +// variable name RenameCounter, and then instrument this renaming with a debug
>> +// counter, like so:
>> +//
>> +// if (!DebugCounter::shouldExecute(RenameCounter)
>> +// <continue or return or whatever not executing looks like>
>> +//
>> +// Now I can, from the command line, make it rename or not rename certain uses
>> +// by setting the skip and count.
>> +// So for example
>> +// bin/opt -debug-counter=predicateinfo-skip=47,predicateinfo-count=1
>> +// will skip renaming the first 47 uses, then rename one, then skip the rest.
>> +
>> +#ifndef LLVM_SUPPORT_DEBUGCOUNTER_H
>> +#define LLVM_SUPPORT_DEBUGCOUNTER_H
>> +
>> +#include "llvm/ADT/DenseMap.h"
>> +#include "llvm/ADT/UniqueVector.h"
>> +#include "llvm/Support/CommandLine.h"
>> +#include "llvm/Support/Debug.h"
>> +#include "llvm/Support/raw_ostream.h"
>> +#include <string>
>> +
>> +namespace llvm {
>> +
>> +class DebugCounter {
>> +public:
>> +  /// \brief Returns a reference to the singleton instance.
>> +  static DebugCounter &instance();
>> +
>> +  // Used by the command line option parser to push a new value it parsed.
>> +  void push_back(const std::string &);
>> +
>> +  // Register a counter with the specified name.
>> +  //
>> +  // FIXME: Currently, counter registration is required to happen before command
>> +  // line option parsing. The main reason to register counters is to produce a
>> +  // nice list of them on the command line, but i'm not sure this is worth it.
>> +  static unsigned registerCounter(StringRef Name, StringRef Desc) {
>> +    return instance().addCounter(Name, Desc);
>> +  }
>> +  inline static bool shouldExecute(unsigned CounterName) {
>> +// Compile to nothing when debugging is off
>> +#ifdef NDEBUG
>> +    return true;
>> +#else
>> +    auto &Us = instance();
>> +    auto Result = Us.Counters.find(CounterName);
>> +    if (Result != Us.Counters.end()) {
>> +      auto &CounterPair = Result->second;
>> +      // We only execute while the skip (first) is zero and the count (second)
>> +      // is non-zero.
>> +      // Negative counters always execute.
>> +      if (CounterPair.first < 0)
>> +        return true;
>> +      if (CounterPair.first != 0) {
>> +        --CounterPair.first;
>> +        return false;
>> +      }
>> +      if (CounterPair.second < 0)
>> +        return true;
>> +      if (CounterPair.second != 0) {
>> +        --CounterPair.second;
>> +        return true;
>> +      }
>> +      return false;
>> +    }
>> +    // Didn't find the counter, should we warn?
>> +    return true;
>> +#endif // NDEBUG
>> +  }
>> +
>> +  // Dump or print the current counter set.
>> +  LLVM_DUMP_METHOD void dump() { print(dbgs()); }
>> +
>> +  void print(raw_ostream &OS);
>> +  
>> +  // Get the counter ID for a given named counter, or return 0 if none is found.
>> +  unsigned getCounterId(const std::string &Name) const {
>> +    return RegisteredCounters.idFor(Name);
>> +  }
>> +
>> +  // Return the number of registered counters.
>> +  unsigned int getNumCounters() const { return RegisteredCounters.size(); }
>> +
>> +  // Return the name and description of the counter with the given ID.
>> +  std::pair<StringRef, StringRef> getCounterInfo(unsigned ID) const {
>> +    return std::make_pair(RegisteredCounters[ID], CounterDesc.lookup(ID));
> 
> DenseMap::lookup returns a copy of the std::string in the map. You cannot return a StringRef pointing to the temp string whose life time ends after this statement.
> 
> Thanks
> 
> Steven
> 
>> +  }
>> +
>> +  // Iterate through the registered counters
>> +  typedef UniqueVector<std::string> CounterVector;
>> +  CounterVector::const_iterator begin() const {
>> +    return RegisteredCounters.begin();
>> +  }
>> +  CounterVector::const_iterator end() const { return RegisteredCounters.end(); }
>> +
>> +private:
>> +  unsigned addCounter(const std::string &Name, const std::string &Desc) {
>> +    unsigned Result = RegisteredCounters.insert(Name);
>> +    CounterDesc[Result] = Desc;
>> +    return Result;
>> +  }
>> +  DenseMap<unsigned, std::pair<long, long>> Counters;
>> +  DenseMap<unsigned, std::string> CounterDesc;
>> +  CounterVector RegisteredCounters;
>> +};
>> +
>> +#define DEBUG_COUNTER(VARNAME, COUNTERNAME, DESC)                              \
>> +  static const unsigned VARNAME =                                              \
>> +      DebugCounter::registerCounter(COUNTERNAME, DESC);
>> +
>> +} // namespace llvm
>> +#endif
>> 
>> Added: llvm/trunk/lib/Support/DebugCounter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/DebugCounter.cpp?rev=295595&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/DebugCounter.cpp?rev=295595&view=auto>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/DebugCounter.cpp (added)
>> +++ llvm/trunk/lib/Support/DebugCounter.cpp Sat Feb 18 22:29:50 2017
>> @@ -0,0 +1,108 @@
>> +#include "llvm/Support/DebugCounter.h"
>> +#include "llvm/Support/CommandLine.h"
>> +#include "llvm/Support/Format.h"
>> +#include "llvm/Support/ManagedStatic.h"
>> +#include "llvm/Support/Options.h"
>> +
>> +using namespace llvm;
>> +
>> +// This class overrides the default list implementation of printing so we
>> +// can pretty print the list of debug counter options.  This type of
>> +// dynamic option is pretty rare (basically this and pass lists).
>> +class DebugCounterList : public cl::list<std::string, DebugCounter> {
>> +private:
>> +  using Base = cl::list<std::string, DebugCounter>;
>> +
>> +public:
>> +  template <class... Mods>
>> +  explicit DebugCounterList(Mods &&... Ms) : Base(std::forward<Mods>(Ms)...) {}
>> +
>> +private:
>> +  void printOptionInfo(size_t GlobalWidth) const override {
>> +    // This is a variant of from generic_parser_base::printOptionInfo.  Sadly,
>> +    // it's not easy to make it more usable.  We could get it to print these as
>> +    // options if we were a cl::opt and registered them, but lists don't have
>> +    // options, nor does the parser for std::string.  The other mechanisms for
>> +    // options are global and would pollute the global namespace with our
>> +    // counters.  Rather than go that route, we have just overridden the
>> +    // printing, which only a few things call anyway.
>> +    outs() << "  -" << ArgStr;
>> +    // All of the other options in CommandLine.cpp use ArgStr.size() + 6 for
>> +    // width, so we do the same.
>> +    Option::printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + 6);
>> +    const auto &CounterInstance = DebugCounter::instance();
>> +    for (auto Name : CounterInstance) {
>> +      const auto Info =
>> +          CounterInstance.getCounterInfo(CounterInstance.getCounterId(Name));
>> +      size_t NumSpaces = GlobalWidth - Info.first.size() - 8;
>> +      outs() << "    =" << Info.first;
>> +      outs().indent(NumSpaces) << " -   " << Info.second << '\n';
>> +    }
>> +  }
>> +};
>> +
>> +// Create our command line option.
>> +static DebugCounterList DebugCounterOption(
>> +    "debug-counter",
>> +    cl::desc("Comma separated list of debug counter skip and count"),
>> +    cl::CommaSeparated, cl::ZeroOrMore, cl::location(DebugCounter::instance()));
>> +
>> +static ManagedStatic<DebugCounter> DC;
>> +
>> +DebugCounter &DebugCounter::instance() { return *DC; }
>> +
>> +// This is called by the command line parser when it sees a value for the
>> +// debug-counter option defined above.
>> +void DebugCounter::push_back(const std::string &Val) {
>> +  if (Val.empty())
>> +    return;
>> +  // The strings should come in as counter=value
>> +  auto CounterPair = StringRef(Val).split('=');
>> +  if (CounterPair.second.empty()) {
>> +    errs() << "DebugCounter Error: " << Val << " does not have an = in it\n";
>> +    return;
>> +  }
>> +  // Now we have counter=value.
>> +  // First, process value.
>> +  long CounterVal;
>> +  if (CounterPair.second.getAsInteger(0, CounterVal)) {
>> +    errs() << "DebugCounter Error: " << CounterPair.second
>> +           << " is not a number\n";
>> +    return;
>> +  }
>> +  // Now we need to see if this is the skip or the count, remove the suffix, and
>> +  // add it to the counter values.
>> +  if (CounterPair.first.endswith("-skip")) {
>> +    auto CounterName = CounterPair.first.drop_back(5);
>> +    unsigned CounterID = RegisteredCounters.idFor(CounterName);
>> +    if (!CounterID) {
>> +      errs() << "DebugCounter Error: " << CounterName
>> +             << " is not a registered counter\n";
>> +      return;
>> +    }
>> +
>> +    auto Res = Counters.insert({CounterID, {-1, -1}});
>> +    Res.first->second.first = CounterVal;
>> +  } else if (CounterPair.first.endswith("-count")) {
>> +    auto CounterName = CounterPair.first.drop_back(6);
>> +    unsigned CounterID = RegisteredCounters.idFor(CounterName);
>> +    if (!CounterID) {
>> +      errs() << "DebugCounter Error: " << CounterName
>> +             << " is not a registered counter\n";
>> +      return;
>> +    }
>> +
>> +    auto Res = Counters.insert({CounterID, {-1, -1}});
>> +    Res.first->second.second = CounterVal;
>> +  } else {
>> +    errs() << "DebugCounter Error: " << CounterPair.first
>> +           << " does not end with -skip or -count\n";
>> +  }
>> +}
>> +
>> +void DebugCounter::print(raw_ostream &OS) {
>> +  OS << "Counters and values:\n";
>> +  for (const auto &KV : Counters)
>> +    OS << left_justify(RegisteredCounters[KV.first], 32) << ": {"
>> +       << KV.second.first << "," << KV.second.second << "}\n";
>> +}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170220/a2ff8a8f/attachment.html>


More information about the llvm-commits mailing list