Project

General

Profile

Actions

Bug #14152

closed

ld(1) should be more careful about empty alists

Added by Rich Lowe about 2 months ago. Updated about 2 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
tools - gate/build tools
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

In various places in the link-editor we check whether a list is NULL to decide whether it is empty (this is, usually, the way the lists work). Unfortunately, this doesn't handle cases where a list has been created but has had something removed from it, we must check they're not NULL and have greater than 0 items.

This, for instance, can cause crashes when placing _end in an object which has had all input files discarded

Actions #1

Updated by Electric Monk about 2 months ago

  • Gerrit CR set to 1747
Actions #2

Updated by Rich Lowe about 2 months ago

testing:
I've tested this with andy's test case and with illumos builds. I can't currently think of anything else to do to aim at specific changed pieces of code (as andy has already proven where we reach first if an item is deleted from the seg list).
The duplicate symbol check is belt and braces, as a non-existed symbol is as impossible to be a duplicate as a lone symbol.

Actions #3

Updated by Rich Lowe about 2 months ago

"andy's test case" was the original source of us knowing about this bug.

For reasons unknown, OmniOS would pass -zignore through LD_OPTIONS, and thus to all ld invocations while building gcc.
This means that GCC's check regarding ld support for merging unlike section attributes would fail, because what the check does is create 3 sections with unlike attributes, but not reference them. What happens next is that because of -zignore processing we discard all 3 sections, and end up with now empty output segments. We crash because we assume the list not being NULL means that it has at least one item in it, and try to place _end relative to nothing.

cat << EOM > conftest1.s
.section myfoosect, "a" 
EOM
cat << EOM > conftest2.s
.section myfoosect, "aw" 
.byte 1
EOM
cat << EOM > conftest3.s
.section myfoosect, "a" 
.byte 0
EOM

obviously discarding these under ignore processing is correct, but equally clearly we must accept that _end can be put somewhere and either way we must not crash. Checking the number of list items, rather than only checking NULL solves this.

(separately, not passing -zignore into checks like this is imperative, since it absolutely thwarts what's being checked)

Actions #4

Updated by Electric Monk about 2 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 8b891ae8e4dfd91d2c49a79262d69a22f346edb6

commit  8b891ae8e4dfd91d2c49a79262d69a22f346edb6
Author: Richard Lowe <richlowe@richlowe.net>
Date:   2021-10-15T23:29:16.000Z

    14152 ld(1) should be more careful about empty alists
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Jason King <jason.brian.king+illumos@gmail.com>
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Approved by: Robert Mustacchi <rm@fingolfin.org>

Actions

Also available in: Atom PDF