Introduce VMCFG_FUZZTESTING

RESOLVED FIXED

Status

Tamarin
Virtual Machine
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Lars T Hansen, Assigned: Lars T Hansen)

Tracking

(Blocks: 1 bug)

Details

(Whiteboard: Has patch)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 years ago
Created attachment 435002 [details] [diff] [review]
Obvious change to avmbuild.h, and one fix for the code

We have occasional informational asserts in the code that trigger in debug builds.  They are used sparingly to indicate that the program - often the host code - is doing something stupid.  The VM will handle the problem but the assert is very useful to alert the programmer that something unusual is happening.

These assertions interfere with fuzz testing.

Propose we introduce VMCFG_FUZZTESTING to wrap around these few asserts, as a lightweight mechanism.  That allows us to fuzz test and expect no assertions.
Attachment #435002 - Flags: review?(edwsmith)
(Assignee)

Updated

8 years ago
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
(Assignee)

Updated

8 years ago
Attachment #435002 - Flags: review?(edwsmith) → review?(rreitmai)

Updated

8 years ago
Attachment #435002 - Flags: review?(rreitmai) → review+

Comment 1

8 years ago
Comment on attachment 435002 [details] [diff] [review]
Obvious change to avmbuild.h, and one fix for the code

Should we consider introducing a -DbeingFuzzed option to toggle these and then add an AvmAssertNonFuzzedMsg()
(Assignee)

Updated

8 years ago
Whiteboard: Has patch
(Assignee)

Comment 2

8 years ago
(In reply to comment #1)
> (From update of attachment 435002 [details] [diff] [review])
> Should we consider introducing a -DbeingFuzzed option to toggle these and then
> add an AvmAssertNonFuzzedMsg()

That might be nice, although the nice thing about the #define is that it can be used in Player builds too, without extra protocol.  With a command line option we have to worry about setting it in a config file, etc.

Maybe some combination, then.
(Assignee)

Comment 3

8 years ago
Comment on attachment 435002 [details] [diff] [review]
Obvious change to avmbuild.h, and one fix for the code

tamarin-redux-argo changeset:   3878:8f4e835b8bef
tamarin-redux changeset:   4190:d515379c772b
(Assignee)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Blocks: 554848

Comment 4

8 years ago
Created attachment 435594 [details] [diff] [review]
Alternate fix: only assert on missing methods for builtin abc's

This is an alternate fix that doesn't need the ifdef:  it's handy to get the assert if the host is doing something stupid, which is indicated by pool->isBuiltin.  When isBuiltin = false, it's a user downloaded abc and we never want the assert.  presumably we will never fuzz the builtin abc's.
Attachment #435594 - Flags: review?(lhansen)
(Assignee)

Updated

8 years ago
Attachment #435594 - Flags: review?(lhansen) → review+

Comment 5

8 years ago
Comment on attachment 435594 [details] [diff] [review]
Alternate fix: only assert on missing methods for builtin abc's

pushed to tr and tr-argo
http://hg.mozilla.org/tamarin-redux/rev/3a3e89c01310
Attachment #435594 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.