-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Null Byte Array Domain #1076
Merged
Merged
Null Byte Array Domain #1076
Changes from 120 commits
Commits
Show all changes
124 commits
Select commit
Hold shift + click to select a range
3ca0419
Merge branch 'base-string-analyses' of github.com:nathanschmidt/gobli…
nathanschmidt 2a53cbb
Very first (incomplete) draft for Must Null Byte Domain
nathanschmidt 7798c04
Draft for complete Null Byte Domain
nathanschmidt d59b45e
Added functions for strstr and str(n)cmp to Null Byte Domain
nathanschmidt 7a41dc4
First adaptations to AttributeConfiguredArrayDomain
nathanschmidt f940d01
Finished draft of Null Byte Array Domain
nathanschmidt 8915ae9
Merge branch 'base-string-analyses' of github.com:nathanschmidt/gobli…
nathanschmidt fa3f7f6
Merge branch 'base-string-analyses' of github.com:nathanschmidt/gobli…
nathanschmidt 9fc814d
Merge branch 'base-string-analyses' of github.com:nathanschmidt/gobli…
nathanschmidt a912463
Addressed github-code-scanning suggestions
nathanschmidt fb65c1c
Fixed integration of domain for base analysis
nathanschmidt b49a043
Fixed incompatible ikinds: changed !Cil.kindOfSizeOf to ILong
nathanschmidt 6023d47
Merge branch 'goblint:master' into null-byte-arrayDomain
nathanschmidt 00941e7
Introduced case for value = bot in make of NullByte
nathanschmidt 85d8ae6
Merge branch 'goblint:master' into null-byte-arrayDomain
nathanschmidt e3a03cd
Merge remote-tracking branch 'origin/null-byte-arrayDomain' into null…
nathanschmidt 03085f5
Handle bot for MustNulls / top for MayNulls properly
nathanschmidt d57ac9e
Fixed usage of domain in base and minor fixes in logic
nathanschmidt 44bd644
Added new thorough regression test
nathanschmidt 472ece8
Feature: better treatment of edge cases
nathanschmidt 6bf2d77
Pass argument to `move_if_affected`
michael-schwarz 60d0687
More missing optional arguments
michael-schwarz 7ebefa8
Merge branch 'goblint:master' into null-byte-arrayDomain
nathanschmidt 3b2f4a5
Fixed integration in base using get thanks to Michael's workaround
nathanschmidt 5873e5f
Tackled feedback: minor improvements and logic fix for not_null
nathanschmidt 40f0de7
Fix macOS tests
nathanschmidt 2072258
Fix test 04-char_arrays.c for macOS
nathanschmidt 9d21da4
Updated is_not_null with case for potential null_ptr
nathanschmidt 780e02a
Update condition for non-zero return by strncmp
nathanschmidt 1bf625d
Fix indentation
nathanschmidt 97cbb4e
Added examples of thesis
nathanschmidt 545714e
Add tests from Juliet
nathanschmidt 0acbf24
Add tests from Juliet
nathanschmidt f4d74e2
Added larger example
nathanschmidt a24546f
Update 07-larger_example.c
nathanschmidt 7dce66d
Merge branch 'goblint:master' into null-byte-arrayDomain
nathanschmidt cc82623
Modification to larger example
nathanschmidt fcd98fa
Merge branch 'null-byte-arrayDomain' of github.com:nathanschmidt/anal…
nathanschmidt b122f4c
Fixed cardinal on top, simplified compute_concat
nathanschmidt 4a088c9
Fixed `content_to_top`
nathanschmidt fd95dbe
Minor bugfix, updated test IDs and annotations
nathanschmidt d0acb0f
Merge branch 'goblint:master' into null-byte-arrayDomain
nathanschmidt e4d7e2b
Fixed test 06 for MacOS
nathanschmidt 0a57374
Make it work with Blobs
michael-schwarz 1aaec46
Update malloced strings destructively where possible
michael-schwarz a0a501c
Replaced type comparison with `CilType.Typ.equal`
nathanschmidt bb373d2
Merge branch 'goblint:master' into null-byte-arrayDomain
nathanschmidt 5dd5da0
Merge branch 'goblint:master' into null-byte-arrayDomain
nathanschmidt fa77d12
Solve failure `Queries.ID.unlift`
nathanschmidt 34c2037
Draft for new regression tests
nathanschmidt e0d9a2a
Updated regression tests
nathanschmidt 5ebd1a1
Bot in string_manipulation: correct ik right away
michael-schwarz d0a90d8
Escape `\0` in XML for g2html compatibility
michael-schwarz 2f7c07f
Add problematic example
michael-schwarz 48d0e5d
Make also fail in the CI
michael-schwarz 5ac2f23
Integrate review suggestions
nathanschmidt c407d3d
Added test cases to increase coverage
nathanschmidt 78d6420
Merge branch 'master' into null-byte-arrayDomain
nathanschmidt 87e76ab
Merge branch 'master' into null-byte-arrayDomain
michael-schwarz c1cced8
Address requested changes to `invalidate_abstract_value`
michael-schwarz e545108
Simplify `substring_extraction`
michael-schwarz 1343915
Some simplifications
michael-schwarz 5f62261
Simplify `AttributeConfiguredAndNullByteArrayDomain`
michael-schwarz a50b1b8
Steps towards simplifications
michael-schwarz 86b7c35
Attempts towards simplification
michael-schwarz a354e63
Simplify
michael-schwarz 8933c0a
Simplify
michael-schwarz 09c069d
Simplify
michael-schwarz f8ee3d2
simplify
michael-schwarz 81c8b63
Cleanup
michael-schwarz 8abc9c9
Progress
michael-schwarz f166671
Simplify
michael-schwarz 404e505
Simplify
michael-schwarz 97c6c08
Simplify
michael-schwarz 92d25b0
Simplify
michael-schwarz 8db2966
Simplify
michael-schwarz 8318ad8
Simplify
michael-schwarz 86872a1
Simplify
michael-schwarz 55d9a53
Simplify
michael-schwarz b4d8bdb
simplify
michael-schwarz 998feb8
Simplify
michael-schwarz 5468275
Simplify
michael-schwarz 23b6f74
SImplify
michael-schwarz 0858696
Progress
michael-schwarz 74c7693
Simplify
michael-schwarz cc90431
Simplify
michael-schwarz cee44cd
Simplify
michael-schwarz 5951b2a
Introduce alias for Z, pull up warning function
michael-schwarz cd57e1f
Progress
michael-schwarz b85ed97
Progress
michael-schwarz ef3f687
Pull things together
michael-schwarz 984165f
Alias for Z.add
michael-schwarz 2135296
More reuse
michael-schwarz 34d2e1c
`to_string` free of direct set manipulation
michael-schwarz df10ad6
Move to operations on Nulls
michael-schwarz 1a0fdb9
Annotate faialing test as TODO
michael-schwarz 2b8e3fa
Simplify
michael-schwarz f51d60f
Simplify
michael-schwarz 0a47ea2
Simplify
michael-schwarz 272e496
Simplify
michael-schwarz 3ebc74d
Remove `idx_maximal` hack
michael-schwarz 63bd31a
Simplify
michael-schwarz 71bce3c
Simplify
michael-schwarz 0b3ff15
Remove `n_exists` construction
michael-schwarz 320cc90
Simplify
michael-schwarz b4bb3c1
Steps towards removing ops on raw sets
michael-schwarz 55a0dd4
Replace exists types with options
michael-schwarz 7a2e9ba
Make types in `string_concat` make sense
michael-schwarz 1282af3
Simplify
michael-schwarz c85bad9
Pull out helper
michael-schwarz 20ee375
One less May/MustSet
michael-schwarz d995cc9
Decouple concrete sets from MaySet
michael-schwarz 72476fb
Simplify
michael-schwarz a73c28d
Lift one more transfer function to work on MustMay
michael-schwarz c15ca04
Use option type
michael-schwarz 1a9ce2c
Strange parens
michael-schwarz 672650b
Merge branch 'master' into null-byte-arrayDomain
michael-schwarz df60d52
Deduplicate ArrayDomain.StrWithDomain declarations
sim642 309f000
Remove trailing whitespace in ArrayDomain
sim642 44705f4
Use ocamldoc references in ArrayDomain.Str
sim642 80c2694
Deduplicate Null declarations
sim642 77b4f67
Fix invalid free in 73-strings/03-string_basics
sim642 f2fdb62
Add TODOs related to null byte array domain
sim642 4577879
Merge branch 'master' into null-byte-arrayDomain
sim642 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1417,7 +1417,7 @@ struct | |
(** [set st addr val] returns a state where [addr] is set to [val] | ||
* it is always ok to put None for lval_raw and rval_raw, this amounts to not using/maintaining | ||
* precise information about arrays. *) | ||
let set (a: Q.ask) ~(ctx: _ ctx) ?(invariant=false) ?lval_raw ?rval_raw ?t_override (gs:glob_fun) (st: store) (lval: AD.t) (lval_type: Cil.typ) (value: value) : store = | ||
let set (a: Q.ask) ~(ctx: _ ctx) ?(invariant=false) ?(blob_destructive=false) ?lval_raw ?rval_raw ?t_override (gs:glob_fun) (st: store) (lval: AD.t) (lval_type: Cil.typ) (value: value) : store = | ||
let update_variable x t y z = | ||
if M.tracing then M.tracel "set" ~var:x.vname "update_variable: start '%s' '%a'\nto\n%a\n\n" x.vname VD.pretty y CPA.pretty z; | ||
let r = update_variable x t y z in (* refers to defintion that is outside of set *) | ||
|
@@ -1450,7 +1450,7 @@ struct | |
let update_offset old_value = | ||
(* Projection globals to highest Precision *) | ||
let projected_value = project_val (Queries.to_value_domain_ask a) None None value (is_global a x) in | ||
let new_value = VD.update_offset (Queries.to_value_domain_ask a) old_value offs projected_value lval_raw ((Var x), cil_offset) t in | ||
let new_value = VD.update_offset ~blob_destructive (Queries.to_value_domain_ask a) old_value offs projected_value lval_raw ((Var x), cil_offset) t in | ||
if WeakUpdates.mem x st.weak then | ||
VD.join old_value new_value | ||
else if invariant then ( | ||
|
@@ -2189,24 +2189,88 @@ struct | |
(* do nothing if all characters are needed *) | ||
| _ -> None | ||
in | ||
let string_manipulation s1 s2 lv all op = | ||
let s1_a, s1_typ = addr_type_of_exp s1 in | ||
let s2_a, s2_typ = addr_type_of_exp s2 in | ||
match lv, op with | ||
| Some lv_val, Some f -> | ||
(* when whished types coincide, compute result of operation op, otherwise use top *) | ||
let lv_a = eval_lv (Analyses.ask_of_ctx ctx) gs st lv_val in | ||
let lv_typ = Cilfacade.typeOfLval lv_val in | ||
if all && typeSig s1_typ = typeSig s2_typ && typeSig s2_typ = typeSig lv_typ then (* all types need to coincide *) | ||
lv_a, lv_typ, (f s1_a s2_a) | ||
else if not all && typeSig s1_typ = typeSig s2_typ then (* only the types of s1 and s2 need to coincide *) | ||
lv_a, lv_typ, (f s1_a s2_a) | ||
else | ||
lv_a, lv_typ, (VD.top_value (unrollType lv_typ)) | ||
| _ -> | ||
(* check if s1 is potentially a string literal as writing to it would be undefined behavior; then return top *) | ||
let _ = AD.string_writing_defined s1_a in | ||
s1_a, s1_typ, VD.top_value (unrollType s1_typ) | ||
let address_from_value (v:value) = match v with | ||
| Address a -> | ||
let rec lo = function | ||
| `Index (i, `NoOffset) -> `NoOffset | ||
| `NoOffset -> `NoOffset | ||
| `Field (f, o) -> `Field (f, lo o) | ||
| `Index (i, o) -> `Index (i, lo o) in | ||
let rmLastOffset = function | ||
| Addr.Addr (v, o) -> Addr.Addr (v, lo o) | ||
| other -> other in | ||
AD.map rmLastOffset a | ||
| _ -> raise (Failure "String function: not an address") | ||
in | ||
let string_manipulation s1 s2 lv all op_addr op_array = | ||
let s1_v = eval_rv (Analyses.ask_of_ctx ctx) gs st s1 in | ||
let s1_a = address_from_value s1_v in | ||
let s1_typ = AD.type_of s1_a in | ||
let s2_v = eval_rv (Analyses.ask_of_ctx ctx) gs st s2 in | ||
let s2_a = address_from_value s2_v in | ||
let s2_typ = AD.type_of s2_a in | ||
(* compute value in string literals domain if s1 and s2 are both string literals *) | ||
if CilType.Typ.equal s1_typ charPtrType && CilType.Typ.equal s2_typ charPtrType then | ||
sim642 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
begin match lv, op_addr with | ||
| Some lv_val, Some f -> | ||
(* when whished types coincide, compute result of operation op_addr, otherwise use top *) | ||
let lv_a = eval_lv (Analyses.ask_of_ctx ctx) gs st lv_val in | ||
let lv_typ = Cilfacade.typeOfLval lv_val in | ||
if all && typeSig s1_typ = typeSig s2_typ && typeSig s2_typ = typeSig lv_typ then (* all types need to coincide *) | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (f s1_a s2_a) | ||
else if not all && typeSig s1_typ = typeSig s2_typ then (* only the types of s1 and s2 need to coincide *) | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (f s1_a s2_a) | ||
else | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (VD.top_value (unrollType lv_typ)) | ||
| _ -> | ||
(* check if s1 is potentially a string literal as writing to it would be undefined behavior; then return top *) | ||
let _ = AD.string_writing_defined s1_a in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st s1_a s1_typ (VD.top_value (unrollType s1_typ)) | ||
end | ||
(* else compute value in array domain *) | ||
else | ||
let lv_a, lv_typ = match lv with | ||
| Some lv_val -> eval_lv (Analyses.ask_of_ctx ctx) gs st lv_val, Cilfacade.typeOfLval lv_val | ||
| None -> s1_a, s1_typ in | ||
begin match (get (Analyses.ask_of_ctx ctx) gs st s1_a None), get (Analyses.ask_of_ctx ctx) gs st s2_a None with | ||
| Array array_s1, Array array_s2 -> set ~ctx ~blob_destructive:true (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (op_array array_s1 array_s2) | ||
| Array array_s1, _ when CilType.Typ.equal s2_typ charPtrType -> | ||
let s2_null_bytes = List.map CArrays.to_null_byte_domain (AD.to_string s2_a) in | ||
let array_s2 = List.fold_left CArrays.join (CArrays.bot ()) s2_null_bytes in | ||
set ~ctx ~blob_destructive:true (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (op_array array_s1 array_s2) | ||
| Bot, Array array_s2 -> | ||
(* If we have bot inside here, we assume the blob is used as a char array and create one inside *) | ||
let ptrdiff_ik = Cilfacade.ptrdiff_ikind () in | ||
let size = ctx.ask (Q.BlobSize {exp = s1; base_address = false}) in | ||
let s_id = | ||
try ValueDomainQueries.ID.unlift (ID.cast_to ptrdiff_ik) size | ||
with Failure _ -> ID.top_of ptrdiff_ik in | ||
let empty_array = CArrays.make s_id (Int (ID.top_of IChar)) in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (op_array empty_array array_s2) | ||
| Bot , _ when CilType.Typ.equal s2_typ charPtrType -> | ||
(* If we have bot inside here, we assume the blob is used as a char array and create one inside *) | ||
let ptrdiff_ik = Cilfacade.ptrdiff_ikind () in | ||
let size = ctx.ask (Q.BlobSize {exp = s1; base_address = false}) in | ||
let s_id = | ||
try ValueDomainQueries.ID.unlift (ID.cast_to ptrdiff_ik) size | ||
with Failure _ -> ID.top_of ptrdiff_ik in | ||
let empty_array = CArrays.make s_id (Int (ID.top_of IChar)) in | ||
let s2_null_bytes = List.map CArrays.to_null_byte_domain (AD.to_string s2_a) in | ||
let array_s2 = List.fold_left CArrays.join (CArrays.bot ()) s2_null_bytes in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (op_array empty_array array_s2) | ||
| _, Array array_s2 when CilType.Typ.equal s1_typ charPtrType -> | ||
(* if s1 is string literal, str(n)cpy and str(n)cat are undefined *) | ||
if op_addr = None then | ||
(* triggers warning, function only evaluated for side-effects *) | ||
let _ = AD.string_writing_defined s1_a in | ||
nathanschmidt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
set ~ctx (Analyses.ask_of_ctx ctx) gs st s1_a s1_typ (VD.top_value (unrollType s1_typ)) | ||
else | ||
let s1_null_bytes = List.map CArrays.to_null_byte_domain (AD.to_string s1_a) in | ||
let array_s1 = List.fold_left CArrays.join (CArrays.bot ()) s1_null_bytes in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (op_array array_s1 array_s2) | ||
| _ -> | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st lv_a lv_typ (VD.top_value (unrollType lv_typ)) | ||
end | ||
in | ||
let st = match desc.special args, f.vname with | ||
| Memset { dest; ch; count; }, _ -> | ||
|
@@ -2229,53 +2293,50 @@ struct | |
set ~ctx (Analyses.ask_of_ctx ctx) gs st dest_a dest_typ value | ||
| Memcpy { dest = dst; src; n; }, _ -> (* TODO: use n *) | ||
memory_copying dst src (Some n) | ||
(* strcpy(dest, src); *) | ||
| Strcpy { dest = dst; src; n = None }, _ -> | ||
let dest_a, dest_typ = addr_type_of_exp dst in | ||
(* when dest surely isn't a string literal, try copying src to dest *) | ||
if AD.string_writing_defined dest_a then | ||
memory_copying dst src None | ||
else | ||
(* else return top (after a warning was issued) *) | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st dest_a dest_typ (VD.top_value (unrollType dest_typ)) | ||
(* strncpy(dest, src, n); *) | ||
| Strcpy { dest = dst; src; n }, _ -> | ||
begin match eval_n n with | ||
| Some num -> | ||
let dest_a, dest_typ, value = string_manipulation dst src None false None in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st dest_a dest_typ value | ||
| None -> failwith "already handled in case above" | ||
end | ||
| Strcat { dest = dst; src; n }, _ -> | ||
let dest_a, dest_typ, value = string_manipulation dst src None false None in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st dest_a dest_typ value | ||
| Strcpy { dest = dst; src; n }, _ -> string_manipulation dst src None false None (fun ar1 ar2 -> Array (CArrays.string_copy ar1 ar2 (eval_n n))) | ||
| Strcat { dest = dst; src; n }, _ -> string_manipulation dst src None false None (fun ar1 ar2 -> Array (CArrays.string_concat ar1 ar2 (eval_n n))) | ||
| Strlen s, _ -> | ||
begin match lv with | ||
| Some lv_val -> | ||
let dest_a = eval_lv (Analyses.ask_of_ctx ctx) gs st lv_val in | ||
let dest_typ = Cilfacade.typeOfLval lv_val in | ||
let lval = mkMem ~addr:(Cil.stripCasts s) ~off:NoOffset in | ||
let address = eval_lv (Analyses.ask_of_ctx ctx) gs st lval in | ||
let (value:value) = Int(AD.to_string_length address) in | ||
let v = eval_rv (Analyses.ask_of_ctx ctx) gs st s in | ||
let a = address_from_value v in | ||
let value:value = | ||
(* if s string literal, compute strlen in string literals domain *) | ||
if AD.type_of a = charPtrType then | ||
Int (AD.to_string_length a) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Polymorphic comparison of types. But also, I'm not sure how reliable this type-based check is because looks like it's trying to check for |
||
(* else compute strlen in array domain *) | ||
else | ||
begin match get (Analyses.ask_of_ctx ctx) gs st a None with | ||
| Array array_s -> Int (CArrays.to_string_length array_s) | ||
| _ -> VD.top_value (unrollType dest_typ) | ||
end in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st dest_a dest_typ value | ||
| None -> st | ||
end | ||
| Strstr { haystack; needle }, _ -> | ||
begin match lv with | ||
| Some _ -> | ||
(* when haystack, needle and dest type coincide, check if needle is a substring of haystack: | ||
if that is the case, assign the substring of haystack starting at the first occurrence of needle to dest, | ||
else use top *) | ||
let dest_a, dest_typ, value = string_manipulation haystack needle lv true (Some (fun h_a n_a -> Address(AD.substring_extraction h_a n_a))) in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st dest_a dest_typ value | ||
| Some lv_val -> | ||
(* check if needle is a substring of haystack in string literals domain if haystack and needle are string literals, | ||
else check in null bytes domain if both haystack and needle are / can be transformed to an array domain representation; | ||
if needle is substring, assign the substring of haystack starting at the first occurrence of needle to dest, | ||
if it surely isn't, assign a null_ptr *) | ||
string_manipulation haystack needle lv true (Some (fun h_a n_a -> Address (AD.substring_extraction h_a n_a))) | ||
(fun h_ar n_ar -> match CArrays.substring_extraction h_ar n_ar with | ||
| CArrays.IsNotSubstr -> Address (AD.null_ptr) | ||
| CArrays.IsSubstrAtIndex0 -> Address (eval_lv (Analyses.ask_of_ctx ctx) gs st (mkMem ~addr:(Cil.stripCasts haystack) ~off:NoOffset)) | ||
| CArrays.IsMaybeSubstr -> Address (AD.join (eval_lv (Analyses.ask_of_ctx ctx) gs st | ||
(mkMem ~addr:(Cil.stripCasts haystack) ~off:(Index (Offset.Index.Exp.any, NoOffset)))) (AD.null_ptr))) | ||
| None -> st | ||
end | ||
| Strcmp { s1; s2; n }, _ -> | ||
begin match lv with | ||
| Some _ -> | ||
(* when s1 and s2 type coincide, compare both both strings completely or their first n characters, otherwise use top *) | ||
let dest_a, dest_typ, value = string_manipulation s1 s2 lv false (Some (fun s1_a s2_a -> Int(AD.string_comparison s1_a s2_a (eval_n n)))) in | ||
set ~ctx (Analyses.ask_of_ctx ctx) gs st dest_a dest_typ value | ||
(* when s1 and s2 are string literals, compare both completely or their first n characters in the string literals domain; | ||
else compare them in the null bytes array domain if they are / can be transformed to an array domain representation *) | ||
string_manipulation s1 s2 lv false (Some (fun s1_a s2_a -> Int (AD.string_comparison s1_a s2_a (eval_n n)))) | ||
(fun s1_ar s2_ar -> Int (CArrays.string_comparison s1_ar s2_ar (eval_n n))) | ||
| None -> st | ||
end | ||
| Abort, _ -> raise Deadcode | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can the last index just be ignored unconditionally. I find it odd because this looks like
Offset.remove_offset
but that one also consistently removes fields.Also, this is just called
address_from_value
and exposed in quite large scope, which is prone to misuse. If the logic is indeed exactly as needed, then this should have a different name or be withintstring_manipulation
or whereever.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that the framework has tacked on a
[0]
offset to string pointer somewhere, but we do want the raw pointer here. You are probably right about naming and moving to a different scope.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be, but there must be a more reliable way to know when this needs to be dropped. Because this also drops such indices when they explicitly occur in the program, rather than being inserted by CIL, and then we would likely do something wrong. For example, what about an array of strings (i.e. multi-dimensional array of
char
s).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not investigated this in detail, but agree someone should ideally do so.