Summary: Another way to schedule work in android is by posting it to a `Handler`. A handler can be constructed out of the main looper, which forces it to schedule work on the UI thread. To model all this, we add syntactic models for getting the main looper and for creating handlers, and dataflow attributes for tracking that an expression is a looper/the main looper, or a handler constructed out of a looper.
Reviewed By: skcho
Differential Revision: D18706768
fbshipit-source-id: 7c46e6913
Summary:
We already have a number of `[pkg].Preconditions.checkNotNull`
modelled, but the one from `androidx` is missing yet widely used.
Reviewed By: mityal
Differential Revision: D18748550
fbshipit-source-id: 83d3317ae
Summary:
The introduction of inferbo intervals as pulse attributes creates the
first relational attributes. To make sense of inferbo intervals
appearing in summaries when in a caller context, we need to substitute
the abstract values they contain in the callee with the abstract values
they correspond to in the caller.
This has a significant consequence: we have to delay the check that
arithmetic constraints in the callee are satisfiable at the call-site
until *after* we have discovered all the relationships between callee
values and caller values from the heap. To solve this, we now run an
arithmetic constraints check *after* having materialised all the
addresses.
We also need to translate the abstract values in the attributes in the
post before recording them in the caller, for the same reasons.
Quite some code in this diff is concerned with substituting pulse values
inside inferbo intervals. There is a complication there too: even after
having discovered relationships between caller and callee abstract
values induced by the heap shapes, there could be abstract values in the
callee's attributes that we haven't seen yet. We need to make up new
values for these in the caller, so this substitution has to return a
potentially extended substitution.
Reviewed By: skcho
Differential Revision: D18745695
fbshipit-source-id: 077ae7670
Summary:
New syntax f1 AND-WITH-WITNESSES f2 : predicate_on_both_witnesses()
This is needed for a linter to check that a macro is present, see the test.
Reviewed By: skcho
Differential Revision: D18735988
fbshipit-source-id: a3be75c5e
Summary:
This gets rid of false positives when something invalid (eg null) is
passed by reference to an initialisation function. Havoc'ing what the
contents of the pointer to results in being optimistic about said
contents in the future.
Also surprisingly gets rid of some FNs (which means it can also
introduce FPs) in the `std::atomic` tests because a path condition
becomes feasible with havoc'ing.
There's a slight refinement possible where we don't havoc pointers to
const but that's more involved and left as future work.
Reviewed By: skcho
Differential Revision: D18726203
fbshipit-source-id: 264b5daeb
Summary:
It's a well-known fact that pulse should know too. To avoid splitting
the abstract state systematically, only act if we know the pointer is
exactly 0 to avoid reporting a nullptr dereference on `free(x)`.
Reviewed By: ezgicicek
Differential Revision: D18708575
fbshipit-source-id: 1cc3f6908
Summary:
Turns out code uses atomics in important places, modelling it removes
FPs.
The tests are copied from biabduction and adapted and extended a bit. I
didn't implement compare_exchange primitives for now (plus, giving them
a sequential semantics like in biabduction is probably a bit cheeky).
Reviewed By: skcho
Differential Revision: D18708576
fbshipit-source-id: a3581b8a4
Summary: This extends semantics of binary operator for BoItv. If there is no known interval value for a pulse value, it returns a symbolic value of the pulse value.
Reviewed By: jvillard
Differential Revision: D18726768
fbshipit-source-id: ed8ecf78b
Summary:
This diff adds inferbo's interval values to pulse's attributes. The added values will be used to
filter out infeasible passes in the following diffs.
Reviewed By: jvillard
Differential Revision: D18726667
fbshipit-source-id: c1125ac6e
Summary:
This will make our analysis more precise. E.g. in the future we can classify
"condition redundand" warnings by origin, and warn only for
InferredNonnull, or autogenerate fixes based on the origin.
Error messaging does not currently take into account origin for nonnull
types, but this can be changed in the future
Reviewed By: ngorogiannis
Differential Revision: D18685304
fbshipit-source-id: 6a1f263f5
Summary: The debugger in the middle of the evaluation makes working on that file difficult. Separating modules a bit, so that we can change the code easier. This is in preparation to trying to add aliases and witnesses to formulas in a next diff.
Reviewed By: jvillard
Differential Revision: D18708542
fbshipit-source-id: 523f30fc9
Summary: The tableaux evaluation was an experiment and it was turned off because of bad perf. Let's kill it to clear up the code.
Reviewed By: jvillard
Differential Revision: D18708388
fbshipit-source-id: 099f5a3d3
Summary:
This diff fixes the model of substring.
Problem: The cost model of the substring function was to return `size of string - start index` as a
cost. However, sometimes this was a negative number, because of state abstractions on paths, array
elements, call contexts, etc, which caused an exception inadvertently.
This diff changes the model to return just `size of string`, when it cannot say `size of string` is
bigger than `start index`.
Reviewed By: ezgicicek
Differential Revision: D18707954
fbshipit-source-id: 63f27e461
Summary: Rather than repeatedly matching actuals, let's use `ProcnameDispatcher.ModeledCall` to pick up the actual arguments with their corresponding values. This simplifies the models.
Reviewed By: jvillard
Differential Revision: D18685855
fbshipit-source-id: 7788bd8bb
Summary: This diff removes `'markers` and `'captured_types` from the procname dispatcher. They are for checking an integrity when a type is captured from template parameters then it is used to match in parameters. However, we have not used that feature, so which simply complicates the types in the dispatcher without any gain at the moment.
Reviewed By: jvillard
Differential Revision: D18706254
fbshipit-source-id: f493778d7
Summary: Preperation diff to use `ProcnameDispatcher` for Pulse: it changes function arguments, i.e. `ProcnameDispatcher.Call.FuncArg`, to a record in order to track the value of arguments. To do that, it changes `ProcnameDispatcher.Call` into a functor so that we can parametrize over the type of the value without making changes upwards.
Reviewed By: jvillard
Differential Revision: D18590224
fbshipit-source-id: 6a13fbc1a
Summary:
This will allow us tune nullsafe behavior in a more fine-grained way.
See e.g. a follow up diff when we report errors more clearly.
Reviewed By: ngorogiannis
Differential Revision: D18683747
fbshipit-source-id: 7b5c42a03
Summary:
We were wrongly not supressing third-party calls in
case third party repo is provided: there was a bug in the function is_modelled_externally.
because of this we accidentally stopped supressing third party and
started to advertise it in non-strict mode.
we do plan doing that, but little bit further ahead :)
So let's add a param guiding this.
Reviewed By: artempyanykh
Differential Revision: D18658271
fbshipit-source-id: 703f2675b
Summary:
We will migrate to the new representation in the future, but for now
let's keep both active.
Reviewed By: artempyanykh
Differential Revision: D18657955
fbshipit-source-id: 9deb03f1f
Summary:
Instead of trying to figure out what runnable is directly passed to an executor,
use attributes to track the flow of a runnable. This has several advantages:
- Can track runnables across function return values.
- Can somewhat overcome the information loss under dynamic dispatch.
- Unifies handling with other attributes.
Reviewed By: skcho
Differential Revision: D18672676
fbshipit-source-id: a06a0e6ff
Summary:
- Unify treatment of modelled and annotated executors by making things go through attributes.
- Add a return attribute to summaries, so that we can track flows of thread guards/executors/future stuff through returned values.
- Dispatch modeled functions to model summaries.
This will help in following diffs where runnables will also go through attributes.
Reviewed By: skcho
Differential Revision: D18660185
fbshipit-source-id: e26b1083e
Summary:
The infer/src/c_stubs directory contains a dune-project file. This
causes ocamlformat to conclude that the root of the project is
infer/src/c_stubs, and there is no .ocamlformat file there, so it does
not apply to the Fnv64Hash.ml file.
(There is currently a bug where it does apply anyway, but it is fixed
in master.)
Reviewed By: dulmarod
Differential Revision: D18684831
fbshipit-source-id: bcb53ce94
Summary:
Some field types of structs are missing in Java. The reason is:
* When capture, empty struct types are added without their fields.
* The empty struct types are overwritten to the global tenv when merging all tenvs.
As a fix, this diff add a boolean field, `dummy`, in `Typ.Struct.t`, then avoids that non-dummy types are replaced by dummy types.
Reviewed By: ngorogiannis
Differential Revision: D18657323
fbshipit-source-id: 4a263f8e7
Summary:
This is a better home for knowing whether a function has sentinel args
according to its prototype declaration.
Reviewed By: dulmarod, artempyanykh
Differential Revision: D18573919
fbshipit-source-id: 13f58eaa2
Summary: Another dead flag that one could mistakenly think is accurate.
Reviewed By: dulmarod
Differential Revision: D18573925
fbshipit-source-id: 129a9cff5
Summary:
This was never set to true except in a wrong way in the Java frontend
(see previous diff).
Reviewed By: dulmarod
Differential Revision: D18573927
fbshipit-source-id: 4c9d1a855
Summary:
This seems just wrong: the assume statement can return if the expression
is true.
Reviewed By: skcho
Differential Revision: D18573921
fbshipit-source-id: 47a5b0ea0
Summary:
A plugin update allows infer to know when a function doesn't return
according to its attributes. This propagates this info all the way to
the attributes of each function, and then use this information in a new
pre-analysis that cuts the links to successor nodes of each `Call`
instruction to a function that does not return.
NOTE: The "no_return" `CallFlag.t` was dead code, following diffs deal
with that (by removing it).
Reviewed By: dulmarod
Differential Revision: D18573922
fbshipit-source-id: 85ec64eca
Summary:
This also prints the CFGs *after* pre-analysis for individual procedures
in infer-out/captured/<filename>/<proc>.dot. One can also look up the
CFGs before pre-analysis in infer-out/captured/proc_cfgs_frontend.dot.
Context: I want to add a pre-analysis that needs to look at proc
attributes inter-procedurally. For this to make sense it has to happen
*after* all of capture, and before analysis.
Thus, this diff brings back the lazy running of the pre-analysis like in
D15803492, except that we still make sure to run the pre-analyses
systematically regardless of the checkers being run by running the
pre-analysis from ondemand.ml. Also we don't need to re-introduce the
"did_preanalysis" proc attribute for the same reason that the
pre-analysis is now run once and for all by ondemand.ml (instead of each
individual checker back in the days).
This has the benefit of running the pre-analysis only when needed, and
the drawback that several concurrent processes analysing the same proc
descs will duplicate work. Since pre-analyses are supposed to be very
fast I assume that neither is a big deal. If they become more expensive
then the benefit gets bigger and the drawback is just the same as with
regular analyses.
Reviewed By: skcho
Differential Revision: D18573920
fbshipit-source-id: de350eaef
Summary: This diff get static value with `EMPTY` field from class initializer.
Reviewed By: ngorogiannis
Differential Revision: D18616588
fbshipit-source-id: 26414c9b2
Summary: When we see a call to schedule some work on an executor and we don't have evidence that it is on some specific thread (UI/BG), instead of dropping the work, assign it `UnknownThread` and treat it as running on the background by default.
Reviewed By: jvillard
Differential Revision: D18615649
fbshipit-source-id: e8bad64b6
Summary:
This allows us to move the CFG rendering to IR/.
The parts of that file concerning CFGs and those concerning Biabduction
specs were entirely disjoint, it turns out, so that was easy.
Reviewed By: jberdine
Differential Revision: D18573924
fbshipit-source-id: 0a5ab6478
Summary: Following D18351867, this diff adds more size alias: when initial array size is one.
Reviewed By: ezgicicek
Differential Revision: D18530598
fbshipit-source-id: 26d57fe30
Summary:
- more flexible API
- less error-prone thanks to named parameters
- also takes care of adjusting predecessors of the previous successors!
This fixes some (probably harmless) bugs in the frontends.
Reviewed By: dulmarod
Differential Revision: D18573923
fbshipit-source-id: ad97b3607
Summary: The transition HOLDS-NEXT WITH-TRANSITION Parameters was only implemented for ObjC methods, now it also works on function calls.
Reviewed By: jvillard
Differential Revision: D18572760
fbshipit-source-id: 06e615cbe
Summary:
Now we point to the root cause of the problem, and also provide
actionable way to solve the issue
Reviewed By: artempyanykh
Differential Revision: D18575650
fbshipit-source-id: ba4884fe1
Summary:
Two goals:
1. Be less assertive when speaking about third party code (it might be
written with different conventions).
2. Point to third party signatures folder so the users know how to
proceed
Reviewed By: artempyanykh
Differential Revision: D18571514
fbshipit-source-id: 854d6e746
Summary:
1/ We now support messaging for third-party: show file name and line
number
2/ We did not show information about internal models in case of param
calls
3/ Small change: we don't specify "modelTables.ml" anymore: no need to
expose implementation details
Reviewed By: artempyanykh
Differential Revision: D18569790
fbshipit-source-id: 28586c8ff
Summary:
In the follow up diff we will use it to render more informative
error message when type error is relevant to the fact that the code is
third-party
Reviewed By: artempyanykh
Differential Revision: D18569783
fbshipit-source-id: 4f6dcc404
Summary:
Let the functions return the modified storage, not unit.
It will play nicely with the next diff, which will add more data to
storage which will be non-mutable.
Reviewed By: artempyanykh
Differential Revision: D18569768
fbshipit-source-id: ade5685be
Summary:
Whole bunch of changes aimed to make error messages more clear and
concise.
1/ Wording and language is unified. We make errors sound more like a
type system violations, rather than linter reccomendations.
Particularly, we refrain from saying things like "may be null" - this is
a linter-style statement that may provoke discussions (what if the
developer knows it can not be null in this particular case).
Instead, we refer to declared nullability and nullability of actual values. This way, it is more clear that this is not a heuristic, this is how rules of a type-system work.
2/ Additionally, we drop things like field class in places when the
context should be clear by who looks at the error. We expect the user
sees the code and the error caption. So e.g. we don't repeat the word "field"
twice.
3/ In cases when we are able to retrieve formal param name, we include it for
usability.
4/ For Field not initialized error, we refer to Initializer methods:
this is a non-obvious but important nullsafe feature.
Reviewed By: artempyanykh
Differential Revision: D18569762
fbshipit-source-id: 9221d7102
Summary:
It make the message bit less heavy, and also it is kind of obvious that
it is origin.
In follow up diffs we will change the text so it is hopefully even more
obvious.
Reviewed By: artempyanykh
Differential Revision: D18527695
fbshipit-source-id: a305d547b
Summary:
1. We don't want to teach the users to ignore noise origin because
sometimes we are going to render something useful for them.
2. It just looks not cool.
Reviewed By: artempyanykh
Differential Revision: D18527694
fbshipit-source-id: 0ea248122
Summary:
By default `install` will always overwrite the destination, which in
particular makes the target looks newer to `make`. This was causing the
models to be rebuilt on every `make`.
Passing `-C` helps with that:
-C, --compare
compare each pair of source and destination files, and in some cases, do not modify the destination at all
Reviewed By: martintrojer
Differential Revision: D18574558
fbshipit-source-id: 40758d689
Summary: Android may spontaneously call these methods on the UI thread, so recognize the fact.
Reviewed By: ezgicicek
Differential Revision: D18530477
fbshipit-source-id: a8a798779
Summary:
First step towards a global analysis. A new command line flag activates the step in `Driver`.
The whole-program analysis is a simple, quadratic (inefficient-as-yet), iteration over all domain elements. However, it is restricted to those elements that are explicitly scheduled to run.
Reviewed By: skcho
Differential Revision: D17787441
fbshipit-source-id: 9fecd766c
Summary:
This diff also removes clumsy typeErr.origin_desc and improves interface
of TypeOrigin.
This will allow to render smarter error message, depending on the
context (see follow up diff).
Reviewed By: ngorogiannis
Differential Revision: D18527692
fbshipit-source-id: b7eb11db8
Summary:
Note: Disabled by default.
Having some support for values, we can report when a null or constant
value is being dereferenced. The particularity here is that we don't
report when 0 is a possible value for the address, or even if we know
that the value of the address can only be 0 in that branch! Instead, we
allow ourselves to report only when we the address has been *set* to
NULL (or any constant).
This is in line with how pulse deals with other issues: only report when
1. we see an address become invalid, and
2. we see the same address be used later on
Reviewed By: skcho
Differential Revision: D17665468
fbshipit-source-id: f1ccf94cf
Summary:
This was causing loads of false positives later in the stack.
Invalidating the address of the object seems to be enough here as it
doesn't break any tests.
Reviewed By: ezgicicek
Differential Revision: D18246090
fbshipit-source-id: 2ef9a6a5c
Summary:
This diff avoids unqualified variables by `ItvUpdatedBy` are qualified later. For example,
```
z = x & y;
z = z + 1;
```
While `z` should not be selected as a control variable, it wasn't, because it was qualified by the addition. This pattern introduces FPs in many cases.
Reviewed By: ngorogiannis
Differential Revision: D18505894
fbshipit-source-id: 13aec3008
Summary:
This diff is a part of work teaching Nullsafe to explain decisions it's
making.
We used to use default for this, which is both buggy and hard to
diagnose. Now we explicitly document nullsafe behaviour in this case.
Reviewed By: artempyanykh
Differential Revision: D18481220
fbshipit-source-id: 0b0cf8b38
Summary:
This diff excludes integer variables from control variables when their values are calculated by
binary operators.
Reviewed By: ezgicicek
Differential Revision: D18505826
fbshipit-source-id: 710533d4c
Summary: Adding a trace that includes when strongSelf was assigned to, and when it's used without a check.
Reviewed By: skcho
Differential Revision: D18506113
fbshipit-source-id: 778c4b086
Summary: Let's make UI/Cold start parts of the message bold to get more attention.
Reviewed By: jvillard
Differential Revision: D18504857
fbshipit-source-id: b0f199f55
Summary:
The variable strongSelf, because it is equal to a weak captured pointer, needs to be checked for null before being used, otherwise it could be null by the time the block is executed.
Added this check to the SelfInBlock checker, and removed it from biabduction.
We want to migrate all the objc checks from biabduction, so it will be easier to change and faster and more reliable. Moreover, this check is more general, it will flag any use of unchecked strongSelf, not just a dereference.
Reviewed By: skcho
Differential Revision: D18403849
fbshipit-source-id: a9cf5d80b
Summary:
There was a precision loss during the substitution of array block. For example:
Callee's abstract memory includes an array block as follows, where `a` is a parameter.
```
a.elements -> { a.elements[*] with a.elements.size }
```
Callers' abstract memory includes a pointer that may point to multiple array blocks.
```
c -> { x, y }
x.elements -> { x.elements[*] with x.elements.size }
y.elements -> { y.elements[*] with y.elements.size }
```
When the callee is called with the parameter `c`, the callees memory is substituted to:
```
x.elements -> { x.elements[*] with top , y.elements[*] with top }
y.elements -> { x.elements[*] with top , y.elements[*] with top }
```
because `a.elements[*]` was substituted to `{ x.elements[*] , y.elements[*] }`
and `a.elements.size` was substituted to `top ( = x.elements.size join y.elements.size )`.
This diff tries to keep the precision in the specific case, not to join the sizes of array blocks.
So now the same callee's abstract memory is substituted to:
```
x.elements -> { x.elements[*] with x.elements.size }
y.elements -> { y.elements[*] with y.elements.size }
```
Reviewed By: ngorogiannis
Differential Revision: D18480585
fbshipit-source-id: b70e63c22
Summary: Due to the weakness of the analysis which can't detect side-effecting prop setting (e.g. as in `builder.prop1(..)`), we currently have many broken chains that do do have any `create` method in their prefixes. Let's not report on these broken chains.
Reviewed By: skcho
Differential Revision: D18503523
fbshipit-source-id: 7506e34b7
Summary:
In some apps executors are obtained by calling standard framework methods (and not by using DI with annotations).
To treat this style, we need to
- Detect calls that return such executors (`do_executor_effect`) and tag the return result with an `Attribute` indicating that it is now an executor, plus what thread it uses.
- Use that information when calling `execute`, to resolve the executor, if any, and its thread (in `do_work_scheduling` via `AttributeDomain.get_executor_constraint`).
- All this requires a new domain component, mapping variables to attributes. This extends the component previously used for remembering whether a variable is the result of a check on whether we run on the UI thread.
At the same time, I un-nested some functions from the transfer function for readability.
Reviewed By: skcho
Differential Revision: D18476122
fbshipit-source-id: bc39b5c2f
Summary:
This diff is a part of work teaching Nullsafe to explain decisions it's
making.
This method was the last escape hatch for InferredNullability to
misreport about its origin.
Now each time we change nullability we provide origin that is consistent
with the nullability itself.
Reviewed By: artempyanykh
Differential Revision: D18453567
fbshipit-source-id: 3d9b7fa2e
Summary:
This is one of key diff in this refactoring stack.
Previously, we used to set nullability independently of type origin,
which opened doors for inconsistent states, bugs, and overcomplicated
code.
Now we have just two method left:
1. `create` -> the main one, yay
2. `set_nonnull` -> will get rid of this in the next diff.
Reviewed By: artempyanykh
Differential Revision: D18451950
fbshipit-source-id: edc485709
Summary:
This diff is a part of work teaching Nullsafe to explain decisions it's
making.
In this diff, we merely rename the value to clearly reflect its currect
usage.
In follow up diffs we are going to make usages of this instance more
restricted and concentrate them in several places in the code.
Reviewed By: artempyanykh
Differential Revision: D18451290
fbshipit-source-id: cf3773364
Summary:
This diff is a part of work teaching Nullsafe to explain decisions it's
making.
1. `Formal` was bit cryptic name.
2. Splitting method param and this makes a lot of sense. It is almost an
implementation detail that hey happen to come from "param" in the
method's signature.
3. Apart from others, this diff fixes a minor bug - we used to treat
this as DeclaredNonnull, which (in future) means suppressing legit warnings
like condition redundant. This would be an issue if we were to start
showing "high confidence" condition redundant warnings.
Reviewed By: ngorogiannis
Differential Revision: D18451294
fbshipit-source-id: acc295e3f
Summary:
This diff is a part of work teaching Nullsafe to explain decisions it's
making.
1. Make Field origin a record: consistent with Proc; more readable; will
add more elements in follow up diffs.
2. `Proc` is bit ambiguous: it is unclear if this is a method call result or param.
Let's make it clear that it is the result of a method call.
Reviewed By: ngorogiannis
Differential Revision: D18450188
fbshipit-source-id: c5abae3ad
Summary:
This diff is a part of work teaching Nullsafe to explain decisions it's
making.
Using `New` was definitely not a good idea here, we need a specific
data type value here.
Reviewed By: artempyanykh
Differential Revision: D18450174
fbshipit-source-id: 7ce7cc7e8
Summary:
This diff is a part of work teaching Nullsafe to explain decisions it's
making.
Introducing more specific origins will allow us to automatically infer
nullability based on type origin (in follow up diffs).
Reviewed By: ngorogiannis
Differential Revision: D18450165
fbshipit-source-id: 9dc0d25c0
Summary:
This diff uses `of_java_array_alloc` instead of `of_c_array_alloc`. Java's array does not have
offset and stride. While this simplifies abstract memories in Java, there is not visible changes I can think of.
Reviewed By: ezgicicek
Differential Revision: D18453474
fbshipit-source-id: 36bdf3daf
Summary:
We were not tracking static `create ` and `build` methods intra-procedurally in the analysis and we just looked up their summaries. This resulted in not having static `create` methods in the trace. Let's fix that by pushing static check into `satisfy_heuristic`.
This results in a few FPs, but such cases are not very common in the codebase.
Reviewed By: skcho
Differential Revision: D18451955
fbshipit-source-id: d4ccf46f5
Summary:
We consider Java collections to be like c++ std::vectors and add models for
- `Collections.get(..)`
- `__cast`
Reviewed By: skcho
Differential Revision: D18449607
fbshipit-source-id: 448206c84
Summary: `equals1` and `equals2` in `SafeInvertedMap.join` are references that indicate whether given parameters and the result is physically equal or not. This diff fixes a missing update of them.
Reviewed By: ezgicicek
Differential Revision: D18450680
fbshipit-source-id: bae19cbe9
Summary:
`getThis` is an idiom for allowing Builder sub-classes to jump through the hoops of covariance plus java generics with self types. It's declared as abstract in the (generic) inner `Builder` class of a root class, and subclasses declare generic `Builder`s that inherit from the generic root `Builder` and trivially implement this method by returning `this`. Obviously, this returns conditional ownership (if `this` is owned, then the return value is owned).
The way it's typically used is
```
T foo() {
...
return getThis();
}
```
However, because abstract methods need dynamic dispatch for proper summarisation, we miss all that. A workaround was been implemented in D8947992 (see that for context), but it was buggy -- it required that the LHS type in the assignment
```
lhs = this.getThis();
```
is the same as the type of `this`, but this is too strict (eg, when using casts).
Here, the condition is changed to requiring that the return type of the method is the same as the type of `this`.
We also avoid asking for the `procdesc` as everything needed is in the attributes.
Reviewed By: jberdine
Differential Revision: D18450737
fbshipit-source-id: e67f0495c
Summary:
It returns non-top value when one of the parameters of band is positive, i.e., `x & 255` returns
`[0, 255]` instead of top.
Reviewed By: ezgicicek
Differential Revision: D18448614
fbshipit-source-id: aaa298a66
Nikos Gorogiannis
Artem Pianykh
Jules Villard
Dulma Churchill
Sungkeun Cho
Mitya Lyubarskiy
Josh Berdine
Ezgi Çiçek
Martin Trojer