Skip to content

Commit

Permalink
Fixed crash in debug when compiling $limit()
Browse files Browse the repository at this point in the history 
Crash took place when compiling $limit().
It was caused by an incorrect assert.
  • Loading branch information
Arpad Bürmen committed Jan 25, 2024
1 parent c0a6778 commit 42026b1
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion openvaf/hir/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl Function {
}

pub fn arg(self, idx: usize, db: &CompilationDB) -> FunctionArg {
debug_assert!(db.function_data(self.id).args.len() <= idx);
debug_assert!(db.function_data(self.id).args.len() >= idx);
FunctionArg { fun_id: self.id, arg_id: idx.into() }
}
pub fn args(self, db: &CompilationDB) -> impl Iterator<Item = FunctionArg> + Clone {
Expand Down

5 comments on commit 42026b1

@gjcoram
Copy link

@gjcoram gjcoram commented on 42026b1 Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does your fix address the issue that $limit(V(a,b)) -- that is, with 1 argument -- will crash? The VAMS LRM only requires one argument, though usually one will supply a function and one or more parameters (eg, "pnjlim" and vcrit).
See #133

@gjcoram
Copy link

@gjcoram gjcoram commented on 42026b1 Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this occur only for $limit using a user-defined function?

@gjcoram
Copy link

@gjcoram gjcoram commented on 42026b1 Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do see a crash when trying to compile the diode example from the Verilog-AMS LRM. It has
analog function real spicepnjlim;
input vnew, vold, vt, vcrit;
and then
vdio = $limit(V(a,c), spicepnjlim, $vt, vcrit);

When I check the values in the assertion, db.function_data(self.id).args.len() is 4 and idx is 0, so the original assertion fails, and the revision passes. I don't understand what this is checking; I would expect a comparison that the call to $limit has the same number of arguments as the user-defined function -- V(a,c) becomes vnew when passed to the function, the second argument is vold instead of the function name, and the last two arguments are passed right over.

@gjcoram
Copy link

@gjcoram gjcoram commented on 42026b1 Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this line:
debug_assert!(db.function_data(self.id).args.len() >= idx);
should instead be
debug_assert!(db.function_data(self.id).args.len() > idx);

As I mentioned in the example above, args.len() is 4, and idx is 0. If I use the patch, then I see a second call with len=4 and idx=1. Since idx is zero-based, I think the inequality should be strictly greater than.

@arpadbuermen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are probably right. Anyway, this is a debug assertion so it does not affect Release version.

Please sign in to comment.