[gcmole] Fixes for unreachable code

tools/gcmole/gcmole.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 330 matching lines @@
 
   typedef std::map<std::string, int> SymbolTable;
 
   bool IsAlive(const std::string& name) const {
     SymbolTable::iterator code = symbol_table_.find(name);
     if (code == symbol_table_.end()) return false;
     return is_live(code->second);
   }
 
   bool Equal(const Environment& env) {
-    if (unreachable_) return env.unreachable_;
+    if (unreachable_ && env.unreachable_) return true;
     size_t size = std::max(live_.size(), env.live_.size());
     for (size_t i = 0; i < size; ++i) {
       if (is_live(i) != env.is_live(i)) return false;
     }
     return true;
   }
 
   Environment Define(const std::string& name) const {
     return Environment(*this, SymbolToCode(name));
   }
@@ skipping 36 matching lines @@
     return allocated_env;
   }
 
  private:
   Environment(const Environment& l, int code)
       : live_(l.live_) {
     set_live(code);
   }
 
   void set_live(size_t pos) {
-    if (live_.size() <= pos) live_.resize(std::max(pos + 1, 2 * live_.size()));
+    if (unreachable_) return;
+    if (pos >= live_.size()) live_.resize(pos + 1);
     live_[pos] = true;
   }
 
   bool is_live(size_t pos) const {
     return unreachable_ || (live_.size() > pos && live_[pos]);
   }
 
   Environment& operator|=(const Environment& o) {
-    unreachable_ |= o.unreachable_;
-    for (size_t i = 0, e = o.live_.size(); i < e; ++i) {
-      if (o.live_[i]) set_live(i);
+    if (o.unreachable_) {
+      unreachable_ = true;
+      live_.clear();
+    } else if (!unreachable_) {
+      for (size_t i = 0, e = o.live_.size(); i < e; ++i) {
+        if (o.live_[i]) set_live(i);
+      }
     }
     return *this;
   }
 
   Environment& operator&=(const Environment& o) {
-    unreachable_ &= o.unreachable_;
-    size_t size = std::min(live_.size(), o.live_.size());
-    if (live_.size() > size) live_.resize(size);
+    if (o.unreachable_) return *this;
+    if (unreachable_) return *this = o;
+
+    size_t size = std::max(live_.size(), o.live_.size());

[Vyacheslav Egorov (Google), 2017/02/16 11:53:00]

I think now that unreachable is handled above it would be correctly to use
min(...) here and < below because whichever of two (|this| or |o|) is longer its
gonna be conjugated with the tail of the smaller one which is all false and
disappear. 
 

[Clemens Hammacher, 2017/02/16 12:02:26]

What if |o| > |this| and the tail of o is all false. Wouldn't we want to resize
|o| to also contain those false values?
I think that's relevant for the Equals operator in the case that one in
unreachable and the other is not. We need to distinguish between false values
and non-existing values, no?

[Vyacheslav Egorov (Google), 2017/02/16 12:17:31]

I see what you mean. Yeah, this subtle. Might warrant a comment here.

[Clemens Hammacher, 2017/02/16 13:13:04]

Added comment.

+    if (size > live_.size()) live_.resize(size);
     for (size_t i = 0; i < size; ++i) {
-      if (live_[i] && (i > o.live_.size() || !o.live_[i])) live_[i] = false;
+      if (live_[i] && (i >= o.live_.size() || !o.live_[i])) live_[i] = false;

[Vyacheslav Egorov (Google), 2017/02/16 11:53:00]

this should just be 

live_[i] = live_[i] && o.live_[i] 

because size would be the minimum of sizes. 

     }
     return *this;
   }
 
   static SymbolTable symbol_table_;
   static std::vector<Environment*> envs_;
 
   std::vector<bool> live_;
+  // unreachable_ == true implies live_.empty(), but still is_live(i) returns
+  // true for all i.
   bool unreachable_ = false;
 
   friend class ExprEffect;
   friend class CallProps;
 };
 
 
 class CallProps {
  public:
   CallProps() : env_(NULL) { }
@@ skipping 831 matching lines @@
 
 
 }
 
 static clang::FrontendPluginRegistry::Add<Action<ProblemsFinder> >
 FindProblems("find-problems", "Find GC-unsafe places.");
 
 static clang::FrontendPluginRegistry::Add<
   Action<FunctionDeclarationFinder> >
 DumpCallees("dump-callees", "Dump callees for each function.");