RTimothyEdwards / magic

Magic VLSI Layout Tool
Other
498 stars 103 forks source link

defRead.c uses function Lookup() where it should rather use LookupFull() #263

Closed chaufe closed 1 year ago

chaufe commented 1 year ago

defRead.c uses function Lookup() for DEF token processing. Lookup() does not only match tokens out of a predefined list of keywords if the match exactly, but also matches tokens that are an abbreviation of keywords as long as the abbreviation is unique. This results in magic supporting DEF files with syntax issues without errors being reported:

Example:

 static char *net_property_keys[] = {
    "USE",
    "ROUTED",
    "NOSHIELD",
    "FIXED",
    "COVER",
    "SOURCE",
    "SHIELDNET",
    "SUBNET",
    "VPIN",
    "XTALK",
    "NONDEFAULTRULE",
    "FIXEDBUMP",
    "FREQUENCY",
    "ORIGINAL",
    "PATTERN",
    "ESTCAP",
    "WEIGHT",
    "PROPERTY",
    NULL
    };

    defLayerMap = defMakeInverseLayerMap(LAYER_MAP_VIAS);

    while ((token = LefNextToken(f, TRUE)) != NULL)
    {
    keyword = Lookup(token, net_keys);
    if (keyword < 0)
    {

This code at line 1031 (function void DefReadNets(...) is supposed to parse net wiring statements. Instead of + ROUTED, it would also support + ROU etc. Switching from Lookup() to LookupFull() would fix this.

Normally, being tolerant on DEF file syntax errors should not be a problem, but in this case, magic just silently exits if a special net containing multiple + FIXED segments will be read-in as a net with one + FIXED segment and the follow-up + FIXED being read-in as + FIXEDBUMP due to this keyword table at line 107 (function DefAddRoutes);

    static char *specnet_keys[] = {
    "SHAPE",
    "STYLE",
    "USE",
    "VOLTAGE",
    "FIXEDBUMP",
    "ORIGINAL",
    "PATTERN",
    "ESTCAP",
    "WEIGHT",
    "PROPERTY",
    NULL
    };

Example code to trigger the issue:

VERSION 5.8 ;
DIVIDERCHAR "/" ;
BUSBITCHARS "[]" ;
DESIGN testcase ;
UNIT DISTANCE MICRONS 2000 ;
DIEAREA ( 0 0 ) ( 100000 100000 ) ;
SPECIALNETS 2 ;
- VDD + USE POWER
  + FIXED Metal4 1000 ( 0 11000 ) ( 100000 * )
  + FIXED Metal5 1000 ( 0 15000 ) ( 100000 * )
  ;
- VSS + USE GROUND
  + ROUTED Metal4 1000 ( 0 21000 ) ( 100000 * )
  + ROUTED Metal5 1000 ( 0 25000 ) ( 100000 * )
  ;

END SPECIALNETS
END DESIGN

If the line containing the second + FIXED statements is removed, magic will complain about the second + ROUTED. Otherwise, it will just silently exit.

/cc @proppy /cc @dhaentz1

RTimothyEdwards commented 1 year ago

@chaufe : Good catch. I'll make the fix right away.

RTimothyEdwards commented 1 year ago

@chaufe : I determined that basically all instances of Lookup() in lefRead.c and defRead.c should be changed to LookupFull(), since Lookup() is, as you point out, only appropriate for command-line parsing within magic's interpreter. I have fixed this in the code source on opencircuitdesign.com (magic version 8.3.425).

chaufe commented 1 year ago

Thanks for the quick fix - tested to be working using magic version 8.3.425.