Closed Bug 1561069 Opened 1 year ago Closed 1 year ago

fix mistranslation in preprocess_libffi_asm.py

Categories

(Firefox Build System :: General, defect)

All
Windows
defect
Not set

Tracking

(firefox69 fixed)

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(1 file)

No description provided.

When we converted our libffi assembly preprocessing to use
GENERATED_FILES, we translated sed 's%/F[dpa][^ ]*%%g' to use the Python
regular expression 'F[dpa][^ ]*'. Note that we missed the leading / in
the sed expression; omitting that leading slash causes no end of trouble if
you have particular expressions in your assembly file, such as
"FastFailCode". (I'm a little surprised this hasn't bitten us yet.)

The straightfoward fix is to add the leading slash.

But wait, the rabbit hole goes deeper. The actual bit from libffi's
msvcc.sh that this was trying to translate was:

    echo "$cl -nologo -EP $includes $defines $src > $ppsrc"
    "$cl" -nologo -EP $includes $defines $src > $ppsrc || exit $?
    output="$(echo $output | sed 's%/F[dpa][^ ]*%%g')"
    args="-nologo $safeseh $single $output $ppsrc"

    echo "$ml $args"
    eval "\"$ml\" $args"
    result=$?

Note that the sed expression is operating on $output, which is a
completely separate thing from the output from the result of
preprocessing. In msvcc.sh, $output is actually some arguments that are
supposed to be passed to the assembler, per the above and the only place in
the script that sets $output to a non-trivial value:

    -o)
      outdir="$(dirname $2)"
      base="$(basename $2|sed 's/\.[^.]*//g')"
      if [ -n "$single" ]; then
        output="-Fo$2"
      else
        output="-Fe$2"
      fi
      if [ -n "$assembly" ]; then
        args="$args $output"
      else
        args="$args $output -Fd$outdir/$base -Fp$outdir/$base -Fa$outdir/$base"
      fi

Presumably the sed expression is attempting to remove -Fd and friends from
$args instead of $output, but failing badly at doing so.

In any event, the regex substitution the script is doing is unnecessary and,
with the current code, actively harmful. Let's remove the regular
expression substitution entirely.

Blocks: 1561088
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1d5517788ccb
fix mistranslation in preprocess_libffi_asm.py; r=glandium
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.