Example build issue using MacOS 12 (Xcode 14.2 from MacOSX.platform).
In file included from grTk1.c:23:
In file included from ../utils/main.h:26:
In file included from ../windows/windows.h:26:
../utils/magic.h:143:13: warning: 'FREAD' macro redefined [-Wmacro-redefined]
#define FREAD(a,b,c,d) gzread(d,a,b*c)
^
/Applications/Xcode_14.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/sys/fcntl.h:110:9: note: previous definition is here
#define FREAD 0x00000001
time.h has existed since C89 so is a standard header expected
to always be available.
sys/time.h was an optional header that historically only some
platforms provided.
If there is a conflict on specific platforms it is better to
'#if !defined()' that specific niche platform with the problem
if both headers are included in the same compile unit. But I
don't think this is a problem in modern times.
So this results in a resolution that removes #ifdef around
time.h and the detection by configure for the availabiltiy
of sys/time.h.
K&R obsolete syntax removal for C23 compatibility series
The local variable 'libnameptr' is used from the 'goto done;' label cleanup
but it may not be initiailzied at the time of the first use of the label.
When evaluating this I also notice the global 'calmaErrorFile' when closed
does not have the handle invalidated.
CalmaRead.c:233:9: warning: variable 'libnameptr' is used uninitialized whenever 'if' condition is true
CalmaRead.c:231:9: warning: variable 'libnameptr' is used uninitialized whenever 'if' condition is true
CalmaRead.c:225:9: warning: variable 'libnameptr' is used uninitialized whenever 'if' condition is true
clang18 -Wall warning cleanup [-Wsometimes-uninitialized]
CalmaRdpt.c:525:5: warning: variable 'rtype' is used uninitialized whenever 'if' condition is true
CalmaRdpt.c:792:5: warning: variable 'rtype' is used uninitialized whenever 'if' condition is true
clang18 -Wall warning cleanup [-Wsometimes-uninitialized]
FEOF condition of PEEKRH causes variable to possibly not get
initialized before use.
CalmaRdcl.c:372:5: warning: variable 'rtype' is used uninitialized whenever 'if' condition is true
clang18 -Wall warning cleanup [-Wsometimes-uninitialized]
Due to use of strcasecmp() or similar C API.
Maybe HAVE_STRINGS_H is needed ? If so which platforms needs this ?
clang18 default warning cleanup (strict)
PlowRules1.c:439:9: warning: suggest parentheses around assignment used as truth value
PlowTech.c:645:17: warning: suggest parentheses around assignment used as truth value
PlowTech.c:652:17: warning: suggest parentheses around assignment used as truth value
PlowTech.c:1019:17: warning: suggest parentheses around assignment used as truth value
ResReadSim.c:270:13: warning: suggest parentheses around assignment used as truth value
ResReadSim.c:871:9: warning: suggest parentheses around assignment used as truth value
ResRex.c:1840:17: warning: suggest parentheses around assignment used as truth value
getrect.c:72:9: warning: suggest parentheses around assignment used as truth value
getrect.c:79:9: warning: suggest parentheses around assignment used as truth value
getrect.c:86:9: warning: suggest parentheses around assignment used as truth value
getrect.c:93:9: warning: suggest parentheses around assignment used as truth value
hash.c:732:16: warning: suggest parentheses around assignment used as truth value
heap.c:328:17: warning: suggest parentheses around assignment used as truth value
heap.c:344:17: warning: suggest parentheses around assignment used as truth value
netlist.c:323:17: warning: suggest parentheses around assignment used as truth value
niceabort.c:121:9: warning: suggest parentheses around assignment used as truth value
path.c:1102:12: warning: suggest parentheses around assignment used as truth value
pathvisit.c:245:13: warning: suggest parentheses around assignment used as truth value
pathvisit.c:295:17: warning: suggest parentheses around assignment used as truth value
tech.c:656:17: warning: suggest parentheses around assignment used as truth value
ext2spice.c:1591:16: warning: suggest parentheses around assignment used as truth value
ext2spice.c:1622:16: warning: suggest parentheses around assignment used as truth value
ext2spice.c:1813:12: warning: suggest parentheses around assignment used as truth value
ext2spice.c:1862:12: warning: suggest parentheses around assignment used as truth value
ext2spice.c:3808:16: warning: suggest parentheses around assignment used as truth value
CalmaRdio.c:437:9: warning: suggest parentheses around assignment used as truth value
CalmaWrite.c:396:9: warning: suggest parentheses around assignment used as truth value
CalmaWrite.c:1772:29: warning: suggest parentheses around assignment used as truth value
CalmaWriteZ.c:372:9: warning: suggest parentheses around assignment used as truth value
CalmaWriteZ.c:1608:29: warning: suggest parentheses around assignment used as truth value
CIFrdtech.c:209:9: warning: suggest parentheses around assignment used as truth value
CIFrdtech.c:214:9: warning: suggest parentheses around assignment used as truth value
CIFrdtech.c:220:9: warning: suggest parentheses around assignment used as truth value
CIFrdtech.c:226:9: warning: suggest parentheses around assignment used as truth value
CIFrdutils.c:1258:12: warning: suggest parentheses around assignment used as truth value
GCC14 -Wall cleanup series [-Wparentheses]
An FEOF exit path exists in READRH() which causes the output
variable(s) to not be assigned a value, then the code makes
a decision (branch) based on uninitialized data.
SonarCloud detection
CalmaRead.c:359:The left operand of '!=' is a garbage value
https://sonarcloud.io/project/issues?open=AZJB17gSNGfDNup0Rkp5&id=dlmiles_magic
The 'predefined' pointer argument to calmaFindCell() is for an optional return value, so must be
NULL when feature is not used.
Copilot Autofix rejected: newdef = calmaFindCell(newname, someSecondArgument);
calma/CalmaRdcl.c
1c822652 (2020-12-04 16:56:51 -0500 1359) bool *predefined; /* If this cell was in memory before the GDS
1c822652 (2020-12-04 16:56:51 -0500 1360) * file was read, then this flag gets set.
1c822652 (2020-12-04 16:56:51 -0500 1361) */
commit 1c82265244 (tag: mpw-one-a, tag: 8.3.92)
Date: Fri Dec 4 16:56:51 2020 -0500
CodeQL: https://github.com/dlmiles/magic/security/code-scanning/6https://github.com/dlmiles/magic/security/code-scanning/5
report it after "Failure to read in entire sub-tree". This will
not report every failing cell (since it quits reading after the
first failure) but will avoid the existing issue of printing
nothing and leaving the user with no feedback as to which cell
was the problem.
"gds readonly true" mode and when writing a GDS file in full-dump
mode. Reading or writing a file with an incompatible DBU is now
prohibited. This is not a great solution, as it forces the
original file to be rewritten with a different DBU. Preferably
there should be code to scale the units during a dump, but that
needs to be coded.
cell being generated. This statement does not disambiguate the
case where a cell is being ripped verbatim from GDS instead of
being generated from the magic database. This print statement
has been split into two cases, and where a cell is being ripped
verbatim, the name of the file is indicated. This provides better
information to the user.
no way to implement boolean operators on labels, so any "label"
statement in the section can apply only to one magic layer. This
is regularly violated in most (all?) techfiles (due mainly to lack
of explanation and guidance). The addition of the "no-reconnect-
labels" option for cifinput made it worse, as it can cause a label
to be attached to the wrong layer and be stuck that way. Even
without the option, an attachment to a non-connecting type is a
problem; DIFF cannot simultaneously have a connection to both
ndiff and pdiff, so it will be one or the other, and the one not
connected can easily get labels moved to other nets. To avoid
this: (1) removed the "no-reconnect-labels" option, and (2) made
the automatic label reconnection smarter, as well as splitting it
into two different behaviors based on whether a label is being
created or manipulated from the command line (more or less the
original behavior) vs. being read from GDS or LEF. The new rules
assume that labels attached to a GDS type will all map to the
same plane in magic. To avoid excessive error messages from
existing tech files, a warning is issued only if "labels" changes
the plane of the target layer (a realistic solution rather than
the preferred one). Also: Fixed an error that causes a crash on
the "wizard" command "*watch" if the cell being observed is
read-only (see github issue #271).
meaning of the MAG record in GDS files. Most available GDS
documentation is decidedly vague about what MAG means. Most
layout tools seem to interpret a MAG of 1 as corresponding to a
text height of 1um. However, there are a few tools that
interpret it as 1 centimicron, and there's no reason to assume
that any given interpretation is correct. "gds magscale" allows
the scale to be redefined.
previous behavior that had inadvertently been changed. In recent
versions, "load <absolute_path> -dereference" would incorrectly
apply the dereferencing to <absolute_path> rather than just its
subcells. Cleaned up the code around DBCellRead() in the process,
so everything is more straightforward (although probably more
could be done in that regard).
to "safer" strncpy() calls to prevent string buffer overflow.
Also: Reimplemented the loop in the GDS write routine that counts
ports and then outputs them in order. It was possible to hang
magic for a long time by giving a port a very, very large index
number. The new implementation uses qsort() to sort the ports
by index, which is obviously much more efficient for the worst
case (and efficient enough for all normal cases).
dereferencing, and making the behavior of "load" on the command
line (i.e., loading a cell from a file) the same as the
behavior of loading a cell as a result of expanding an unloaded
instance. In both cases, if "load -dereference" is used, and
a cell does not exist in any search path but does exist in the
original location, without dereferencing, then the cell will be
loaded from the original location. Also: Corrected an error
that has existed since adding the capability to read compressed
files, which causes magic to crash when attempting to run the
"crash recover" command (because that routine was mixing
compressed and regular file stream calls).
ground node name (which is static) gets put on the node list and
is improperly deallocated. Corrected by simply allocating the
string for the default substrate node instead of using the static
string.
"cif *hier write disable" and "cif *array write disable" commands
for a specific cell def and its descendents. The revision ensures
that all descendents apply the override. The "cif write" command
uses a stack instead of recursion, which makes it difficult to
apply the same method. Currently the method only works for the
"gds write" command, and implementing the feature for "cif write"
is deemed not worth the effort.
This is diagnostic only and does not change the read-in
behavior.
(2) ext2spice: Corrected an error that had been introduced
into version 8.3.171 that accidentally marks all devices
as visited which causes all source/drain areas and
perimeters to be output as zero.
(3) extract: Sweeping changes to handling of fringe
capacitance. Removed the (recently added) "fringeshieldhalo"
parameter from the tech file. Reworked the fringe
capacitance models based on results from the "capiche"
project (github/RTimothyEdwards/capiche). Fringe shielding
is now done by clipping fringe at the boundary of a
shielding shape, rather than trying to calculate the
amount of shielding (as the "capiche" project proved this
to be equivalent). Values for partial fringing are modeled
by atan(x), which like the sidewall (1/x) curve, extends to
infinity and values are limited by the halo but do not
otherwise depend on the halo. Because of this, the halo can
be made variable and controlled by the user for deciding on
the tradeoff between accuracy and run time. A new command
option "extract halo" was added to allow this control over
the halo distance.
three types: "none", "temporary", and "keep" (instead of "true"
or "false"). "none" now reverts back to the original behavior,
because it was found that saving polygons in subcells prevents
them from participating in boolean operations. The "keep"
option is the original option (polygons kept in subcells), and
"temporary" is the one recently introduced (which puts polygons
in subcells and then flattens them). This restores the original
method while retaining the recently implemented method. However,
a proper solution needs to be found that deals with the problem
of boolean operators.
tech file to correct the underlying problem with the SkyWater
sky130 process in which a different layer/purpose pair is used
for TEXTTYPE and DATATYPE for the same layer. Previously, all
output from magic writes the same pair for both when writing a
port label. The new method preserves existing syntax, although
there are some differences based on what order the "port"
statement appears relative to other types for the same layer.
that the feature for implementing callbacks on a selection list
was already implemented via the add_dependency procedure.
Modified the GDS read to remove cell instances that are placed
directly on top of one another in the same cell. Modified the
GDS read to make a better selection of a default font size for
text that specifies a font but not a size, using the minimum
width for the layer the text is placed on. Modified the GDS
read to remove text with empty-string placeholders (created when
a pin layer is read but no text exists to go along with it, due
to GDS not having a specific way to make pins, such that pins
have to be split between one record for geometry and another
for text).
should have been done a long time ago! Allows an instance to be
flattened in place inside a cell def, which otherwise requires
a complicated set of commands to do. Also: Modified the polygon
handling routine from the previous commit so that it correctly
removes the polygon cell defs after flattening them into the
parent cell.
can be much, much faster than reading in polygons directly into a
cell from GDS. Modified the handling of polygons so that they are
*always* read into subcells. If the "polygon subcell flatten true"
option is not enabled, then the subcells are flattened at the end
of reading the cell, and the polygon cells are deleted. This method
avoids most of the cases in which "polygon subcell flatten true"
has ever needed to be set.
due to a missing function prototype. Modified the GDS output
flow to always output instance IDs as a property, not just in the
non-default case. The property has been used for many years and
appears to be accepted by all tools reading GDS, so there is no
downside to always generating this output. This has the upside
that default instance names don't get scrambled by going from
magic to GDS and back to magic.
Darryl Miles
Tim Edwards
Johan Euphrosine