Skip to content

Conversation

@myxoid
Copy link
Owner

@myxoid myxoid commented Jul 15, 2020

Attempt to increase determinism

@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 0e301d6 to 3b2ca8b Compare July 16, 2020 17:20
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 3b2ca8b to 4847c98 Compare July 23, 2020 18:22
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 4847c98 to ccc7db0 Compare July 27, 2020 10:50
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from ccc7db0 to 85b67d7 Compare July 27, 2020 18:21
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch 3 times, most recently from e2380ad to b58f9b6 Compare August 5, 2020 06:52
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from b58f9b6 to 618c199 Compare August 7, 2020 14:15
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 618c199 to 6481f26 Compare September 15, 2020 14:15
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 6481f26 to 63d6a22 Compare September 29, 2020 15:40
myxoid added a commit that referenced this pull request Sep 29, 2020
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 63d6a22 to 82f711f Compare October 2, 2020 07:02
myxoid added a commit that referenced this pull request Oct 6, 2020
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 82f711f to a21788a Compare October 6, 2020 20:59
myxoid added a commit that referenced this pull request Nov 6, 2020
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from a21788a to 28fa545 Compare November 6, 2020 12:53
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 28fa545 to c87fdc7 Compare November 16, 2020 11:22
myxoid added a commit that referenced this pull request Nov 16, 2020
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
myxoid added a commit that referenced this pull request Nov 30, 2020
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from c87fdc7 to 7b0a968 Compare November 30, 2020 18:48
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 7b0a968 to 1a8af70 Compare February 16, 2021 18:29
myxoid added a commit that referenced this pull request Feb 16, 2021
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
myxoid added a commit that referenced this pull request Mar 23, 2021
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 1a8af70 to 465038c Compare March 23, 2021 16:25
myxoid added a commit that referenced this pull request Mar 31, 2021
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch 2 times, most recently from 2094939 to 8bfc4c0 Compare April 6, 2021 21:15
myxoid added a commit that referenced this pull request Apr 6, 2021
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 8bfc4c0 to e5a8ecd Compare April 16, 2021 10:15
myxoid added a commit that referenced this pull request Apr 16, 2021
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from e5a8ecd to be29c60 Compare May 7, 2021 18:38
myxoid added a commit that referenced this pull request May 7, 2021
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from be29c60 to a7f2353 Compare May 26, 2021 14:46
myxoid added a commit that referenced this pull request May 26, 2021
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
myxoid added a commit that referenced this pull request Jun 11, 2021
Ignore this commit message, it's all been squashed. The intermediate
commits no longer passed the test suite, perhaps due to types where
the recursion ended up with a type on the left side of an equality
later appearing on the right.

------------------------------>CUT<------------------------------

> This is the 1st commit message:

Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

> This is the commit message #2:

abg-ir.cc: Adjust assertions around being-compared types

This commit adjust the assertions in the code that tracks in-progress
type comparisons. The mark/unmark operations should be strictly paired
and it should never be possible to pass a null pointer.

	* src/abg-ir.cc (unmark_as_being_compared): Adjust doc comment
	to reflect that the methods expect to find the type marked. In
	the overload that does the removal, assert that this happened.
	In the overload taking a plain pointer, remove the null
	pointer test. (comparison_started): In the class_or_union
        overload taking a plain pointer, remove the null pointer test.
        (mark_as_being_compared): Do something else. TBcompleted.

Signed-off-by: Giuliano Procida <gprocida@google.com>

> This is the commit message #3:

Mark/unmark/test comparisons part 1

> This is the commit message #4:

Record both types being compared.

The current code that compares types has protection against infinite
recursion. It works as follows.

For types which can introduce recursion (function types - hang on, can
they really? - and class/union types), a set of currently currently
being compared types is maintained and if a type is seen again, the
equality test automatically short-cuts to true.

The tests use of set containing both the left and right side of the
comparison and checks test for either. The difference between this and
the more correct insertion and checking of pairs may be nil or
non-existent but the storage requirements are likely to be almost
identical.

The bracketed push/pop behaviour means we can assert stronger
invariants on the code and only need to manipulate the top of a
stack. Using a vector over a set likely has a performance advantage as
the number of elements is typically very small.

- max size in test suite 10 pairs
- max size in kernel ABI 6 pairs
- typical max size 2-3 pairs

This commit switches from sets of single pointers to vectors of pairs
of pointers.

Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from a7f2353 to 030d7a3 Compare June 11, 2021 15:10
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 030d7a3 to 8b71a6d Compare November 18, 2021 17:12
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 8b71a6d to b8ff538 Compare January 20, 2022 13:30
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from b8ff538 to 6b09fb7 Compare February 8, 2022 11:51
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from 6b09fb7 to f598d26 Compare March 10, 2022 17:23
myxoid added 3 commits June 2, 2022 12:57
This commit switches all the annotation tests and most of the DWARF
reader tests to output with hash-based type ids.

This is to make later code changes that affect ABI output easier
to review.

The first nine DWARF reader tests remain duplicated with both
SEQUENCE_TYPE_ID_STYLE and HASH_TYPE_ID_STYLE.

Signed-off-by: Giuliano Procida <gprocida@google.com>
A common source of kernel ABI churn is the movement of type declarations
between abi-instr XML elements.

While it may be possible to fix this in the longer term by moving
types to a dedicated section of ABI XML, a short-term fix would make
reviewing ABI changes less painful.

It is suspected that translation unit order within a corpus may
contribute to this issue. This commit imposes a deterministic order.

Some of the variations in test output may point to latent bugs.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
@myxoid myxoid force-pushed the dwarf-reader-sort-translation-units branch from f598d26 to 9827eef Compare June 2, 2022 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants