GP-2424 Decompiler allows unknown prototype model names

Ghidra/Features/Decompiler/src/decompile/cpp/architecture.cc

@@ -221,9 +221,9 @@ Architecture::~Architecture(void)
 
 /// The Architecture maintains the set of prototype models that can
 /// be applied for this particular executable. Retrieve one by name.
-/// The model must exist or an exception is thrown.
+/// If the model doesn't exist, null is returned.
 /// \param nm is the name
-/// \return the matching model
+/// \return the matching model or null
 ProtoModel *Architecture::getModel(const string &nm) const
 
 {
@@ -231,7 +231,7 @@ ProtoModel *Architecture::getModel(const string &nm) const
 
   iter = protoModels.find(nm);
   if (iter==protoModels.end())
-    throw LowlevelError("Prototype model does not exist: "+nm);
+    return (ProtoModel *)0;
   return (*iter).second;
 }
 
@@ -313,10 +313,13 @@ int4 Architecture::getMinimumLanedRegisterSize(void) const
 /// The default model is used whenever an explicit model is not known
 /// or can't be determined.
 /// \param nm is the name of the model to set
-void Architecture::setDefaultModel(const string &nm)
+void Architecture::setDefaultModel(ProtoModel *model)
 
 {
-  defaultfp = getModel(nm);
+  if (defaultfp != (ProtoModel *)0)
+    defaultfp->setPrintInDecl(true);
+  model->setPrintInDecl(false);
+  defaultfp = model;
 }
 
 /// Throw out the syntax tree, (unlocked) symbols, comments, and other derived information
@@ -817,7 +820,7 @@ ProtoModel *Architecture::decodeProto(Decoder &decoder)
 
   res->decode(decoder);
   
-  ProtoModel *other = protoModels[res->getName()];
+  ProtoModel *other = getModel(res->getName());
   if (other != (ProtoModel *)0) {
     string errMsg = "Duplicate ProtoModel name: " + res->getName();
     delete res;
@@ -836,7 +839,7 @@ void Architecture::decodeProtoEval(Decoder &decoder)
 {
   uint4 elemId = decoder.openElement();
   string modelName = decoder.readString(ATTRIB_NAME);
-  ProtoModel *res = protoModels[ modelName ];
+  ProtoModel *res = getModel(modelName);
   if (res == (ProtoModel *)0)
     throw LowlevelError("Unknown prototype model name: "+modelName);
 
@@ -864,7 +867,8 @@ void Architecture::decodeDefaultProto(Decoder &decoder)
   while(decoder.peekElement() != 0) {
     if (defaultfp != (ProtoModel *)0)
       throw LowlevelError("More than one default prototype model");
-    defaultfp = decodeProto(decoder);
+    ProtoModel *model = decodeProto(decoder);
+    setDefaultModel(model);
   }
   decoder.closeElement(elemId);
 }
@@ -1192,6 +1196,20 @@ void Architecture::createModelAlias(const string &aliasName,const string &parent
   protoModels[aliasName] = new ProtoModel(aliasName,*model);
 }
 
+/// A new UnknownProtoModel, which clones its behavior from the default model, is created and associated with the
+/// unrecognized name.  Subsequent queries of the name return this new model.
+/// \param modelName is the unrecognized name
+/// \return the new \e unknown prototype model associated with the name
+ProtoModel *Architecture::createUnknownModel(const string &modelName)
+
+{
+  UnknownProtoModel *model = new UnknownProtoModel(modelName,defaultfp);
+  protoModels[modelName] = model;
+  if (modelName == "unknown")		// "unknown" is a reserved/internal name
+    model->setPrintInDecl(false);	// don't print it in declarations
+  return model;
+}
+
 /// This looks for the \<processor_spec> tag and and sets configuration
 /// parameters based on it.
 /// \param store is the document store holding the tag
@@ -1360,8 +1378,8 @@ void Architecture::parseCompilerConfig(DocumentStorage &store)
   addOtherSpace();
       
   if (defaultfp == (ProtoModel *)0) {
-    if (protoModels.size() == 1)
+    if (protoModels.size() > 0)
-      defaultfp = (*protoModels.begin()).second;
+      setDefaultModel((*protoModels.begin()).second);
     else
       throw LowlevelError("No default prototype specified");
   }

Ghidra/Features/Decompiler/src/decompile/cpp/architecture.hh

@@ -216,11 +216,12 @@ public:
   void resetDefaults(void);		///< Reset defaults values for options owned by \b this
   ProtoModel *getModel(const string &nm) const;		///< Get a specific PrototypeModel
   bool hasModel(const string &nm) const;		///< Does this Architecture have a specific PrototypeModel
+  ProtoModel *createUnknownModel(const string &modelName);	///< Create a model for an unrecognized name
   bool highPtrPossible(const Address &loc,int4 size) const; ///< Are pointers possible to the given location?
   AddrSpace *getSpaceBySpacebase(const Address &loc,int4 size) const; ///< Get space associated with a \e spacebase register
   const LanedRegister *getLanedRegister(const Address &loc,int4 size) const;	///< Get LanedRegister associated with storage
   int4 getMinimumLanedRegisterSize(void) const;		///< Get the minimum size of a laned register in bytes
-  void setDefaultModel(const string &nm);		///< Set the default PrototypeModel
+  void setDefaultModel(ProtoModel *model);		///< Set the default PrototypeModel
   void clearAnalysis(Funcdata *fd);			///< Clear analysis specific to a function
   void readLoaderSymbols(const string &delim);		 ///< Read any symbols from loader into database
   void collectBehaviors(vector<OpBehavior *> &behave) const;	///< Provide a list of OpBehavior objects

Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc

@@ -2174,7 +2174,7 @@ int4 ActionDefaultParams::apply(Funcdata &data)
 
       if (otherfunc != (Funcdata *)0) {
 	fc->copy(otherfunc->getFuncProto());
-	if ((!fc->isModelLocked())&&(!fc->hasMatchingModel(evalfp)))
+	if ((!fc->isModelLocked())&& !fc->hasMatchingModel(evalfp))
 	  fc->setModel(evalfp);
       }
       else
@@ -4060,7 +4060,7 @@ int4 ActionPrototypeTypes::apply(Funcdata &data)
   ProtoModel *evalfp = data.getArch()->evalfp_current;
   if (evalfp == (ProtoModel *)0)
     evalfp = data.getArch()->defaultfp;
-  if ((!data.getFuncProto().isModelLocked())&&(!data.getFuncProto().hasMatchingModel(evalfp)))
+  if ((!data.getFuncProto().isModelLocked()) && !data.getFuncProto().hasMatchingModel(evalfp))
     data.getFuncProto().setModel(evalfp);
   if (data.getFuncProto().hasThisPointer())
     data.prepareThisPointer();
@@ -4340,9 +4340,14 @@ int4 ActionPrototypeWarnings::apply(Funcdata &data)
   if (ourproto.hasOutputErrors()) {
     data.warningHeader("Cannot assign location of return value for this function: Return value may be inaccurate");
   }
-  if (ourproto.isUnknownModel() && (!ourproto.hasCustomStorage()) &&
+  if (ourproto.isModelUnknown()) {
-  	(ourproto.isInputLocked() || ourproto.isOutputLocked())) {
+    ostringstream s;
-    data.warningHeader("Unknown calling convention yet parameter storage is locked");
+    s << "Unknown calling convention";
+    if (ourproto.printModelInDecl())
+      s << ": " << ourproto.getModelName();
+    if (!ourproto.hasCustomStorage() && (ourproto.isInputLocked() || ourproto.isOutputLocked()))
+      s << " -- yet parameter storage is locked";
+    data.warningHeader(s.str());
  }
   int4 numcalls = data.numCalls();
   for(int4 i=0;i<numcalls;++i) {

Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc

@@ -2104,6 +2104,7 @@ ProtoModel::ProtoModel(Architecture *g)
   stackgrowsnegative = true;	// Normal stack parameter ordering
   hasThis = false;
   isConstruct = false;
+  isPrinted = true;
   defaultLocalRange();
   defaultParamRange();
 }
@@ -2116,6 +2117,7 @@ ProtoModel::ProtoModel(const string &nm,const ProtoModel &op2)
 {
   glb = op2.glb;
   name = nm;
+  isPrinted = true;		// Don't inherit. Always print unless setPrintInDecl called explicitly
   extrapop = op2.extrapop;
   if (op2.input != (ParamList *)0)
     input = op2.input->clone();
@@ -2302,6 +2304,7 @@ void ProtoModel::decode(Decoder &decoder)
   extrapop = -300;
   hasThis = false;
   isConstruct = false;
+  isPrinted = true;
   effectlist.clear();
   injectUponEntry = -1;
   injectUponReturn = -1;
@@ -3546,7 +3549,6 @@ void FuncProto::setModel(ProtoModel *m)
     model = m;
     extrapop = ProtoModel::extrapop_unknown;
   }
-  flags &= ~((uint4)unknown_model);	// Model is not "unknown" (even if null pointer is passed in)
 }
 
 /// The full function prototype is (re)set from a model, names, and data-types
@@ -4374,7 +4376,6 @@ void FuncProto::decode(Decoder &decoder,Architecture *glb)
     throw LowlevelError("Prototype storage must be set before restoring FuncProto");
   ProtoModel *mod = (ProtoModel *)0;
   bool seenextrapop = false;
-  bool seenunknownmod = false;
   int4 readextrapop;
   flags = 0;
   injectid = -1;
@@ -4384,14 +4385,13 @@ void FuncProto::decode(Decoder &decoder,Architecture *glb)
     if (attribId == 0) break;
     if (attribId == ATTRIB_MODEL) {
       string modelname = decoder.readString();
-      if ((modelname == "default")||(modelname.size()==0))
+      if (modelname.size()==0 || modelname == "default")
-	mod = glb->defaultfp;	// Get default model
+	mod = glb->defaultfp;	// Use the default model
-      else if (modelname == "unknown") {
+      else {
-	mod = glb->defaultfp;		// Use the default
-	seenunknownmod = true;
-      }
-      else
 	mod = glb->getModel(modelname);
+	if (mod == (ProtoModel *)0)	// Model name is unrecognized
+	  mod = glb->createUnknownModel(modelname);	// Create model with placeholder behavior
+      }
     }
     else if (attribId == ATTRIB_EXTRAPOP) {
       seenextrapop = true;
@@ -4438,8 +4438,6 @@ void FuncProto::decode(Decoder &decoder,Architecture *glb)
     setModel(mod);		// This sets extrapop to model default
   if (seenextrapop)		// If explicitly set
     extrapop = readextrapop;
-  if (seenunknownmod)
-    flags |= unknown_model;
 
   uint4 subId = decoder.peekElement();
   if (subId != 0) {

Ghidra/Features/Decompiler/src/decompile/cpp/fspec.hh

@@ -715,6 +715,7 @@ class ProtoModel {
   bool stackgrowsnegative;	///< True if stack parameters have (normal) low address to high address ordering
   bool hasThis;			///< True if this model has a \b this parameter (auto-parameter)
   bool isConstruct;		///< True if this model is a constructor for a particular object
+  bool isPrinted;		///< True if this model should be printed as part of function declarations
   void defaultLocalRange(void);	///< Set the default stack range used for local variables
   void defaultParamRange(void);	///< Set the default stack range used for input parameters
   void buildParamList(const string &strategy);	 ///< Establish the main resource lists for input and output parameters.
@@ -927,6 +928,8 @@ public:
   bool isStackGrowsNegative(void) const { return stackgrowsnegative; }	///< Return \b true if the stack \e grows toward smaller addresses
   bool hasThisPointer(void) const { return hasThis; }			///< Is \b this a model for (non-static) class methods
   bool isConstructor(void) const { return isConstruct; }		///< Is \b this model for class constructors
+  bool printInDecl(void) const { return isPrinted; }	///< Return \b true if name should be printed in function declarations
+  void setPrintInDecl(bool val) { isPrinted = val; }	///< Set whether \b this name should be printed in function declarations
 
   /// \brief Return the maximum heritage delay across all possible input parameters
   ///
@@ -945,11 +948,26 @@ public:
   int4 getMaxOutputDelay(void) const { return output->getMaxDelay(); }
 
   virtual bool isMerged(void) const { return false; }	///< Is \b this a merged prototype model
+  virtual bool isUnknown(void) const { return false; }	///< Is \b this an unrecognized prototype model
   virtual void decode(Decoder &decoder);		///< Restore \b this model from a stream
   static uint4 lookupEffect(const vector<EffectRecord> &efflist,const Address &addr,int4 size);
   static int4 lookupRecord(const vector<EffectRecord> &efflist,int4 listSize,const Address &addr,int4 size);
 };
 
+/// \brief An unrecognized prototype model
+///
+/// This kind of model is created for function prototypes that specify a model name for which
+/// there is no matching object.  A model is created for the name by cloning behavior from a
+/// placeholder model, usually the \e default model.  This object mostly behaves like its placeholder
+/// model but can identify itself as an \e unknown model and adopts the unrecognized model name.
+class UnknownProtoModel : public ProtoModel {
+  ProtoModel *placeholderModel;		///< The model whose behavior \b this adopts as a behavior placeholder
+public:
+  UnknownProtoModel(const string &nm,ProtoModel *placeHold) : ProtoModel(nm,*placeHold) { placeholderModel = placeHold; }
+  ProtoModel *getPlaceholderModel(void) const { return placeholderModel; }	///< Retrieve the placeholder model
+  virtual bool isUnknown(void) const { return true; }
+};
+
 /// \brief Class for calculating "goodness of fit" of parameter trials against a prototype model
 ///
 /// The class is instantiated with a prototype model (ProtoModel). A set of Varnode parameter trials
@@ -1280,11 +1298,10 @@ class FuncProto {
     error_inputparam = 64,	///< Set if the input parameters are not properly represented
     error_outputparam = 128,	///< Set if the return value(s) are not properly represented
     custom_storage = 256,	///< Parameter storage is custom (not derived from ProtoModel)
-    unknown_model = 512,	///< Set if the PrototypeModel isn't known
+    is_constructor = 0x200,	///< Function is an (object-oriented) constructor
-    is_constructor = 0x400,	///< Function is an (object-oriented) constructor
+    is_destructor = 0x400,	///< Function is an (object-oriented) destructor
-    is_destructor = 0x800,	///< Function is an (object-oriented) destructor
+    has_thisptr= 0x800,		///< Function is a method with a 'this' pointer as an argument
-    has_thisptr= 0x1000,	///< Function is a method with a 'this' pointer as an argument
+    is_override = 0x1000	///< Set if \b this prototype is created to override a single call site
-    is_override = 0x2000	///< Set if \b this prototype is created to override a single call site
   };
   ProtoModel *model;		///< Model of for \b this prototype
   ProtoStore *store;		///< Storage interface for parameters
@@ -1317,15 +1334,15 @@ public:
   void setModel(ProtoModel *m);						///< Set the prototype model for \b this
   bool hasModel(void) const { return (model != (ProtoModel *)0); }	///< Does \b this prototype have a model
 
-  bool hasMatchingModel(const FuncProto *op2) const { return (model == op2->model); }	///< Does \b this have a matching model
   bool hasMatchingModel(const ProtoModel *op2) const { return (model == op2); }	///< Does \b this use the given model
   const string &getModelName(void) const { return model->getName(); }	///< Get the prototype model name
   int4 getModelExtraPop(void) const { return model->getExtraPop(); }	///< Get the \e extrapop of the prototype model
+  bool isModelUnknown(void) const { return model->isUnknown(); }	///< Return \b true if the prototype model is \e unknown
+  bool printModelInDecl(void) const { return model->printInDecl(); }	///< Return \b true if the name should be printed in declarations
 
   bool isInputLocked(void) const;					///< Are input data-types locked
   bool isOutputLocked(void) const { return store->getOutput()->isTypeLocked(); }	///< Is the output data-type locked
   bool isModelLocked(void) const { return ((flags&modellock)!=0); }	///< Is the prototype model for \b this locked
-  bool isUnknownModel(void) const { return ((flags&unknown_model)!=0); }	///< Is prototype model officially "unknown"
   bool hasCustomStorage(void) const { return ((flags&custom_storage)!=0); }	///< Is \b this a "custom" function prototype
   void setInputLock(bool val);				///< Toggle the data-type lock on input parameters
   void setOutputLock(bool val);				///< Toggle the data-type lock on the return value

Ghidra/Features/Decompiler/src/decompile/cpp/options.cc

@@ -257,7 +257,10 @@ string OptionReadOnly::apply(Architecture *glb,const string &p1,const string &p2
 string OptionDefaultPrototype::apply(Architecture *glb,const string &p1,const string &p2,const string &p3) const
 
 {
-  glb->setDefaultModel(p1);
+  ProtoModel *model = glb->getModel(p1);
+  if (model == (ProtoModel *)0)
+    throw LowlevelError("Unknown prototype model :" + p1);
+  glb->setDefaultModel(model);
   return "Set default prototype to "+p1;
 }
 
@@ -764,7 +767,7 @@ string OptionProtoEval::apply(Architecture *glb,const string &p1,const string &p
   if (p1 == "default")
     model = glb->defaultfp;
   else {
-    model = glb->protoModels[p1];
+    model = glb->getModel(p1);
     if (model == (ProtoModel *)0)
       throw ParseError("Unknown prototype model: "+p1);
   }

Ghidra/Features/Decompiler/src/decompile/cpp/prettyprint.cc

@@ -37,16 +37,6 @@ ElementId ELEM_VARIABLE = ElementId("variable",26);
 
 const string Emit::EMPTY_STRING = "";
 
-const string EmitMarkup::highlight[] = {	"keyword",
-					"comment",
-					"type",
-					 "funcname",
-					 "var",
-					 "const",
-					 "param",
-					 "global",
-					  "" };
-
 /// \brief Emit a sequence of space characters as part of source code
 ///
 /// \param num is the number of space characters to emit
@@ -161,7 +151,7 @@ void EmitMarkup::tagVariable(const string &name,syntax_highlight hl,const Varnod
 {
   encoder->openElement(ELEM_VARIABLE);
   if (hl != no_color)
-    encoder->writeString(ATTRIB_COLOR, highlight[(int4)hl]);
+    encoder->writeUnsignedInteger(ATTRIB_COLOR, hl);
   if (vn != (const Varnode *)0)
     encoder->writeUnsignedInteger(ATTRIB_VARREF, vn->getCreateIndex());
   if (op != (const PcodeOp *)0)
@@ -175,7 +165,7 @@ void EmitMarkup::tagOp(const string &name,syntax_highlight hl,const PcodeOp *op)
 {
   encoder->openElement(ELEM_OP);
   if (hl != no_color)
-    encoder->writeString(ATTRIB_COLOR,highlight[(int4)hl]);
+    encoder->writeUnsignedInteger(ATTRIB_COLOR,hl);
   if (op != (const PcodeOp *)0)
     encoder->writeUnsignedInteger(ATTRIB_OPREF, op->getTime());
   encoder->writeString(ATTRIB_CONTENT,name);
@@ -187,7 +177,7 @@ void EmitMarkup::tagFuncName(const string &name,syntax_highlight hl,const Funcda
 {
   encoder->openElement(ELEM_FUNCNAME);
   if (hl != no_color)
-    encoder->writeString(ATTRIB_COLOR,highlight[(int4)hl]);
+    encoder->writeUnsignedInteger(ATTRIB_COLOR,hl);
   if (op != (const PcodeOp *)0)
     encoder->writeUnsignedInteger(ATTRIB_OPREF, op->getTime());
   encoder->writeString(ATTRIB_CONTENT,name);
@@ -199,7 +189,7 @@ void EmitMarkup::tagType(const string &name,syntax_highlight hl,const Datatype *
 {
   encoder->openElement(ELEM_TYPE);
   if (hl != no_color)
-    encoder->writeString(ATTRIB_COLOR,highlight[(int4)hl]);
+    encoder->writeUnsignedInteger(ATTRIB_COLOR,hl);
   if (ct->getId() != 0) {
     encoder->writeUnsignedInteger(ATTRIB_ID, ct->getId());
   }
@@ -212,7 +202,7 @@ void EmitMarkup::tagField(const string &name,syntax_highlight hl,const Datatype
 {
   encoder->openElement(ELEM_FIELD);
   if (hl != no_color)
-    encoder->writeString(ATTRIB_COLOR,highlight[(int4)hl]);
+    encoder->writeUnsignedInteger(ATTRIB_COLOR,hl);
   if (ct != (const Datatype *)0) {
     encoder->writeString(ATTRIB_NAME,ct->getName());
     if (ct->getId() != 0) {
@@ -231,7 +221,7 @@ void EmitMarkup::tagComment(const string &name,syntax_highlight hl,const AddrSpa
 {
   encoder->openElement(ELEM_COMMENT);
   if (hl != no_color)
-    encoder->writeString(ATTRIB_COLOR,highlight[(int4)hl]);
+    encoder->writeUnsignedInteger(ATTRIB_COLOR,hl);
   encoder->writeSpace(ATTRIB_SPACE, spc);
   encoder->writeUnsignedInteger(ATTRIB_OFF, off);
   encoder->writeString(ATTRIB_CONTENT,name);
@@ -243,7 +233,7 @@ void EmitMarkup::tagLabel(const string &name,syntax_highlight hl,const AddrSpace
 {
   encoder->openElement(ELEM_LABEL);
   if (hl != no_color)
-    encoder->writeString(ATTRIB_COLOR,highlight[(int4)hl]);
+    encoder->writeUnsignedInteger(ATTRIB_COLOR,hl);
   encoder->writeSpace(ATTRIB_SPACE,spc);
   encoder->writeUnsignedInteger(ATTRIB_OFF, off);
   encoder->writeString(ATTRIB_CONTENT,name);
@@ -255,7 +245,7 @@ void EmitMarkup::print(const string &data,syntax_highlight hl)
 {
   encoder->openElement(ELEM_SYNTAX);
   if (hl != no_color)
-    encoder->writeString(ATTRIB_COLOR,highlight[(int4)hl]);
+    encoder->writeUnsignedInteger(ATTRIB_COLOR,hl);
   encoder->writeString(ATTRIB_CONTENT,data);
   encoder->closeElement(ELEM_SYNTAX);
 }

Ghidra/Features/Decompiler/src/decompile/cpp/prettyprint.hh

@@ -103,6 +103,8 @@ protected:
 public:
   Emit(void) { indentlevel=0; parenlevel=0; pendPrint=(PendPrint *)0; resetDefaultsInternal(); }	///< Constructor
   /// \brief Possible types of syntax highlighting
+  ///
+  /// Values must match constants in ClangToken
   enum syntax_highlight {
     keyword_color = 0,		///< Keyword in the high-level language
     comment_color = 1,		///< Comments
@@ -112,7 +114,8 @@ public:
     const_color = 5,		///< Constant values
     param_color = 6,		///< Function parameters
     global_color = 7,		///< Global variable identifiers
-    no_color = 8		///< Un-highlighted
+    no_color = 8,		///< Un-highlighted
+    error_color = 9		///< Indicates a warning or error state
   };
   virtual ~Emit(void) {}				///< Destructor
 
@@ -437,7 +440,6 @@ public:
 /// This class can be used as the low-level back-end to EmitPrettyPrint to provide a solution
 /// that does both pretty printing and markup.
 class EmitMarkup : public Emit {
-  static const string highlight[];	///< Map from syntax_highlight enumeration to color attribute string
 protected:
   ostream *s;				///< Stream being emitted to
   Encoder *encoder;			///< How markup is encoded to the output stream

Ghidra/Features/Decompiler/src/decompile/cpp/printc.cc

@@ -2479,13 +2479,12 @@ void PrintC::emitFunctionDeclaration(const Funcdata *fd)
   emitPrototypeOutput(proto,fd);
   emit->spaces(1);
   if (option_convention) {
-    if (fd->getFuncProto().hasModel()) {
+    if (fd->getFuncProto().printModelInDecl()) {
-      if (!fd->getFuncProto().hasMatchingModel(fd->getArch()->defaultfp)) { // If not the default
+      Emit::syntax_highlight highlight = fd->getFuncProto().isModelUnknown() ? Emit::error_color : Emit::keyword_color;
-	emit->print(fd->getFuncProto().getModelName(),EmitMarkup::keyword_color);
+      emit->print(fd->getFuncProto().getModelName(),highlight);
       emit->spaces(1);
     }
   }
-  }
   int4 id1 = emit->openGroup();
   emitSymbolScope(fd->getSymbol());
   emit->tagFuncName(fd->getName(),EmitMarkup::funcname_color,fd,(PcodeOp *)0);

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/ClangMarkup.java

@@ -25,16 +25,6 @@ import ghidra.program.model.pcode.*;
 
 public abstract class ClangMarkup {			// Placeholder for CLANG XML identifiers
 
-// Attribute values
-	public static final String KEYWORD_COLOR = "keyword";
-	public static final String COMMENT_COLOR = "comment";
-	public static final String TYPE_COLOR = "type";
-	public static final String FUNCNAME_COLOR = "funcname";
-	public static final String VARIABLE_COLOR = "var";
-	public static final String CONST_COLOR = "const";
-	public static final String PARAMETER_COLOR = "param";
-	public static final String GLOBAL_COLOR = "global";
-
 	public static ClangTokenGroup buildClangTree(Decoder decoder, HighFunction hfunc)
 			throws DecoderException {
 		ClangTokenGroup docroot;

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/ClangSyntaxToken.java

@@ -42,8 +42,8 @@ public class ClangSyntaxToken extends ClangToken {
 		open = close = -1;
 	}
 
-	public ClangSyntaxToken(ClangNode par, String txt, String col) {
+	public ClangSyntaxToken(ClangNode par, String txt, int color) {
-		super(par, txt, col);
+		super(par, txt, color);
 		open = close = -1;
 	}
 

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/ClangToken.java

@@ -38,15 +38,17 @@ import ghidra.program.model.pcode.*;
  * May contain links back to pcode object
  */
 public class ClangToken implements ClangNode {
-	public final static int KEYWORD_COLOR = 0;
+	public final static int KEYWORD_COLOR = 0;	// Constants must match Decompiler syntax_highlight
-	public final static int TYPE_COLOR = 1;
+	public final static int COMMENT_COLOR = 1;
-	public final static int FUNCTION_COLOR = 2;
+	public final static int TYPE_COLOR = 2;
-	public final static int COMMENT_COLOR = 3;
+	public final static int FUNCTION_COLOR = 3;
 	public final static int VARIABLE_COLOR = 4;
 	public final static int CONST_COLOR = 5;
 	public final static int PARAMETER_COLOR = 6;
 	public final static int GLOBAL_COLOR = 7;
 	public final static int DEFAULT_COLOR = 8;
+	public final static int ERROR_COLOR = 9;
+	public final static int MAX_COLOR = 10;
 
 	private ClangNode parent;
 	private ClangLine lineparent;
@@ -59,7 +61,7 @@ public class ClangToken implements ClangNode {
 		parent = par;
 		text = null;
 		highlight = null;
-		syntax_type = getColor(null);
+		syntax_type = DEFAULT_COLOR;
 		lineparent = null;
 	}
 
@@ -67,14 +69,14 @@ public class ClangToken implements ClangNode {
 		parent = par;
 		text = txt;
 		highlight = null;
-		syntax_type = getColor(null);
+		syntax_type = DEFAULT_COLOR;
 	}
 
-	public ClangToken(ClangNode par, String txt, String col) {
+	public ClangToken(ClangNode par, String txt, int color) {
 		parent = par;
 		text = txt;
 		highlight = null;
-		syntax_type = getColor(col);
+		syntax_type = color;
 	}
 
 	@Override
@@ -156,19 +158,21 @@ public class ClangToken implements ClangNode {
 	}
 
 	public void decode(Decoder decoder, PcodeFactory pfactory) throws DecoderException {
-		String col = null;
+		syntax_type = DEFAULT_COLOR;
 		for (;;) {
 			int attribId = decoder.getNextAttributeId();
 			if (attribId == 0) {
 				break;
 			}
 			if (attribId == ATTRIB_COLOR.id()) {
-				col = decoder.readString();
+				syntax_type = (int) decoder.readUnsignedInteger();
 				break;
 			}
 		}
 		text = decoder.readString(ATTRIB_CONTENT);
-		syntax_type = getColor(col);
+		if (syntax_type < 0 || syntax_type >= MAX_COLOR) {
+			syntax_type = DEFAULT_COLOR;
+		}
 	}
 
 	@Override
@@ -213,36 +217,6 @@ public class ClangToken implements ClangNode {
 		return token;
 	}
 
-	public static int getColor(String col) {
-		if (col != null) {
-			if (col.equals(ClangMarkup.KEYWORD_COLOR)) {
-				return KEYWORD_COLOR;
-			}
-			else if (col.equals(ClangMarkup.VARIABLE_COLOR)) {
-				return VARIABLE_COLOR;
-			}
-			else if (col.equals(ClangMarkup.CONST_COLOR)) {
-				return CONST_COLOR;
-			}
-			else if (col.equals(ClangMarkup.PARAMETER_COLOR)) {
-				return PARAMETER_COLOR;
-			}
-			else if (col.equals(ClangMarkup.GLOBAL_COLOR)) {
-				return GLOBAL_COLOR;
-			}
-			else if (col.equals(ClangMarkup.TYPE_COLOR)) {
-				return TYPE_COLOR;
-			}
-			else if (col.equals(ClangMarkup.COMMENT_COLOR)) {
-				return COMMENT_COLOR;
-			}
-			else if (col.equals(ClangMarkup.FUNCNAME_COLOR)) {
-				return FUNCTION_COLOR;
-			}
-		}
-		return DEFAULT_COLOR; // The default color
-	}
-
 	static public ClangToken buildSpacer(ClangNode par, int indent, String indentStr) {
 		String spacing = new String();
 		for (int i = 0; i < indent; ++i) {

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java

@@ -347,6 +347,9 @@ public class DecompileOptions {
 	private static final Color SEARCH_HIGHLIGHT_DEF = new Color(100, 100, 255);
 	private Color defaultSearchHighlightColor = SEARCH_HIGHLIGHT_DEF;
 
+	// Color applied to a token to indicate warning/error
+	private final static Color ERROR_COLOR = new Color(204, 0, 51);     // Dark Red
+
 	final static String FONT_MSG = "Display.Font";
 	final static Font DEFAULT_FONT = new Font(Font.MONOSPACED, Font.PLAIN, 12);
 	private Font defaultFont;
@@ -798,54 +801,100 @@ public class DecompileOptions {
 		this.maxwidth = maxwidth;
 	}
 
+	/**
+	 * @return color associated with keyword tokens
+	 */
 	public Color getKeywordColor() {
 		return keywordColor;
 	}
 
+	/**
+	 * @return color associated with data-type tokens
+	 */
 	public Color getTypeColor() {
 		return typeColor;
 	}
 
+	/**
+	 * @return color associated with a function name token
+	 */
 	public Color getFunctionColor() {
 		return functionColor;
 	}
 
+	/**
+	 * @return color used to display comments
+	 */
 	public Color getCommentColor() {
 		return commentColor;
 	}
 
+	/**
+	 * @return color associated with constant tokens
+	 */
 	public Color getConstantColor() {
 		return constantColor;
 	}
 
+	/**
+	 * @return color associated with (local) variable tokens
+	 */
 	public Color getVariableColor() {
 		return variableColor;
 	}
 
+	/**
+	 * @return color associated with parameter tokens
+	 */
 	public Color getParameterColor() {
 		return parameterColor;
 	}
 
+	/**
+	 * @return color associated with global variable tokens
+	 */
 	public Color getGlobalColor() {
 		return globalColor;
 	}
 
+	/**
+	 * @return color for generic syntax or other unspecified tokens
+	 */
 	public Color getDefaultColor() {
 		return defaultColor;
 	}
 
+	/**
+	 * @return color used on tokens that need to warn of an error or other unusual conditions 
+	 */
+	public Color getErrorColor() {
+		return ERROR_COLOR;
+	}
+
+	/**
+	 * @return the background color for the decompiler window
+	 */
 	public Color getCodeViewerBackgroundColor() {
 		return codeViewerBackgroundColor;
 	}
 
+	/**
+	 * @return the color used display the current highlighted variable
+	 */
 	public Color getCurrentVariableHighlightColor() {
 		return currentVariableHighlightColor;
 	}
 
+	/**
+	 * @return color used to highlight token(s) selected with a middle button clock
+	 */
 	public Color getMiddleMouseHighlightColor() {
 		return middleMouseHighlightColor;
 	}
 
+	/**
+	 * @return color used to highlight search results
+	 */
 	public Color getSearchHighlightColor() {
 		return defaultSearchHighlightColor;
 	}

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/ClangLayoutController.java

@@ -75,7 +75,7 @@ public class ClangLayoutController implements LayoutModel, LayoutModelListener {
 			FontMetrics met, HighlightFactory hlFactory) {
 		options = opt;
 		this.decompilerPanel = decompilerPanel;
-		syntax_color = new Color[9];
+		syntax_color = new Color[ClangToken.MAX_COLOR];
 		metrics = met;
 		this.hlFactory = hlFactory;
 		EMPTY_LINE_NUMBER_SPACER = createEmptyLineNumberSpacer();
@@ -239,6 +239,7 @@ public class ClangLayoutController implements LayoutModel, LayoutModelListener {
 		syntax_color[ClangToken.PARAMETER_COLOR] = options.getParameterColor();
 		syntax_color[ClangToken.GLOBAL_COLOR] = options.getGlobalColor();
 		syntax_color[ClangToken.DEFAULT_COLOR] = options.getDefaultColor();
+		syntax_color[ClangToken.ERROR_COLOR] = options.getErrorColor();
 
 		// setting the metrics here will indirectly trigger the new font to be used deeper in
 		// the bowels of the FieldPanel (you can get the font from the metrics)
@@ -340,7 +341,7 @@ public class ClangLayoutController implements LayoutModel, LayoutModelListener {
 			ClangTokenGroup line = new ClangTokenGroup(docroot);
 			ClangBreak lineBreak = new ClangBreak(line, 1);
 			ClangSyntaxToken message =
-				new ClangSyntaxToken(line, errline, ClangMarkup.COMMENT_COLOR);
+				new ClangSyntaxToken(line, errline, ClangToken.COMMENT_COLOR);
 			line.AddTokenGroup(lineBreak);
 			line.AddTokenGroup(message);
 			docroot.AddTokenGroup(line);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/FunctionPrototype.java

@@ -90,9 +90,9 @@ public class FunctionPrototype {
 	 */
 	public FunctionPrototype(FunctionSignature proto, CompilerSpec cspec,
 			boolean voidimpliesdotdotdot) {
+		modelname = proto.getGenericCallingConvention().getDeclarationName();
 		PrototypeModel model = cspec.matchConvention(proto.getGenericCallingConvention());
 		localsyms = null;
-		modelname = model.getName();
 		gconv = proto.getGenericCallingConvention();
 		injectname = null;
 		returntype = proto.getReturnType();
@@ -425,10 +425,7 @@ public class FunctionPrototype {
 		modelname = decoder.readString(ATTRIB_MODEL);
 		PrototypeModel protoModel =
 			dtmanage.getProgram().getCompilerSpec().getCallingConvention(modelname);
-		if (protoModel == null) {
+		hasThis = (protoModel == null) ? false : protoModel.hasThisPointer();
-			throw new DecoderException("Bad prototype model name: " + modelname);
-		}
-		hasThis = protoModel.hasThisPointer();
 		try {
 			extrapop = (int) decoder.readSignedInteger(ATTRIB_EXTRAPOP);
 		}