Improper usage of CHECK macros generates unreachable code

[Christian Thalinger]

On Wed, 2009-10-07 at 20:20 +1000, David Holmes - Sun Microsystems
wrote:
> I dont think it sufficient to simply replace CHECK with THREAD. The 
> problem is that the value being returned may be complete garbage. ie given:
> 
>      return create_string_variable(ns, name, 0, s, CHECK);
> 
> the intent is to return NULL if create_string_variable triggers an 
> exception, but if we change this to:
> 
>      return create_string_variable(ns, name, 0, s, THREAD);
> 
> we don't know what value will be returned without examining the 
> internals of create_string_variable. So the replacement would be unsafe 
> unless we verified that create_string_variable always returns NULL if it 
> detects an exception is generated, but if that were the case there would 
> be no need to use the CHECK macro in the first place. We may find that 
> we are returning a garbage value.

That's true.  But the current CHECK actually does what THREAD would do,
given the macro expands to:

  static PerfStringVariable* create_string_variable(CounterNS ns,
                                                    const char* name,
                                                    const char *s, TRAPS) {
    return create_string_variable(ns, name, 0, s, THREAD);
    if (HAS_PENDING_EXCEPTION) return NULL; (0);
  };

I don't see how we would not return garbage here too.  So we have to
store the return value in temporary variable, as suggested by Volker.

-- Christian