Skip to content
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

Compile errors #2

Closed
viccherubini opened this issue Feb 20, 2010 · 8 comments
Closed

Compile errors #2

viccherubini opened this issue Feb 20, 2010 · 8 comments

Comments

@viccherubini
Copy link

I am able to successfully configure the project (cmake . works fine, it says success), I have the patched libraries installed, however, I get compile errors whenever I try to make the project.

I get to about 12% of the entire build it and it dies. Here is the entire build trace: http://qst.me/7b0

And the output from cmake: http://qst.me/87c

Any help would be appreciated. Thanks!

@scottmac
Copy link
Contributor

What sort of system do you have?

gcc --version
g++ --version
uname -a

Any other information is useful

@viccherubini
Copy link
Author

gcc --version
gcc (Ubuntu 4.4.1-4ubuntu9) 4.4.1

g++ --version
g++ (Ubuntu 4.4.1-4ubuntu9) 4.4.1

uname -a
Linux ralph 2.6.31-19-generic #56-Ubuntu SMP Thu Jan 28 01:26:53 UTC 2010 i686 GNU/Linux

Running Ubuntu 32bit Desktop, I'll try Server in a moment.

Could it be some redeclaration of ssize_t within libafdt? I'm really not all versed in C/C++ development on a Unix environment.

Thanks!

@viccherubini
Copy link
Author

And /proc/cpuinfo
http://qst.me/390

@scottmac
Copy link
Contributor

All of our machines are 64-bit, there is a chance its not 32-bit safe. Which is the opposite problem of most people.

@viccherubini
Copy link
Author

Ahh, I have a 64bit VM, I'll try installing it there and seeing how that goes. Thanks for the help.

@fgm
Copy link

fgm commented Feb 20, 2010

I tried commenting out that part just to see if it went further, but there are similar overload failures (same platform) further down, still in the zend_*.cpp files.

@carsonmcdonald
Copy link
Contributor

I can confirm that a 32 bit Fedora 12 install can't compile the code but 64 bit Fedora 12 can.

@scottmac
Copy link
Contributor

We're only supporting 64-bit at the moment.

scottmac pushed a commit that referenced this issue May 27, 2011
Summary:
The newly added test case failed because Variant::setNull calls unset
which leaves an uninitialized NULL which is wrong. I inlined the code
and set to KindOfNull.

Test Plan:
The new test case output:
Before
  object(A)#2 (0) {
  }
  HipHop Notice:  Undefined property: B::$data
  NULL
After
  object(A)#2 (0) {
  }
  NULL

make fast_tests
make slow_tests

Reviewed By: mwilliams
Reviewers: mwilliams
CC: hphp-diffs@lists, ps, mwilliams
Revert Plan:
Tags:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Differential Revision: 258084
inez pushed a commit to inez/hiphop-php that referenced this issue Jun 8, 2011
Summary:
The newly added test case failed because Variant::setNull calls unset
which leaves an uninitialized NULL which is wrong. I inlined the code
and set to KindOfNull.

Test Plan:
The new test case output:
Before
  object(A)facebook#2 (0) {
  }
  HipHop Notice:  Undefined property: B::$data
  NULL
After
  object(A)facebook#2 (0) {
  }
  NULL

make fast_tests
make slow_tests

Reviewed By: mwilliams
Reviewers: mwilliams
CC: hphp-diffs@lists, ps, mwilliams
Revert Plan:
Tags:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Differential Revision: 258084
h4ck3rm1k3 pushed a commit to h4ck3rm1k3/hhvm that referenced this issue Sep 13, 2011
Summary:
declared those new functions and constants, then

 diff facebook#2 will have a naive implementation, copying entire global states and
input parameters, nullifying all fiber-unsafe extension objects.

 diff facebook#3 will experiment adding fiber Variant wrapper to do copy-on-writes

Test Plan:
nothing outstanding in this diff

DiffCamp Revision: 119354
Reviewed By: iproctor
CC: hphp-diffs@lists, iproctor
Tasks:

Revert Plan:
OK
h4ck3rm1k3 pushed a commit to h4ck3rm1k3/hhvm that referenced this issue Sep 13, 2011
…) with deep copy

Summary:
I'll have diff facebook#2-2 to handle global variables and ThreadLocal<T>s, including
ExecutionContext and extension's thread locals.

Test Plan:
the new unit tests, nothing existing is altered

DiffCamp Revision: 120935
Reviewed By: iproctor
CC: hphp-diffs@lists, iproctor, hzhao
Tasks:
#141384: add multi-threading support to PHP

Revert Plan:
OK
h4ck3rm1k3 pushed a commit to h4ck3rm1k3/hhvm that referenced this issue Sep 13, 2011
Summary:
Implement the support of the runtime otpion Log.InjectedStackTrace. When it is
turned on, the log will include stacktrace generated from frame injection.

Test Plan:
<?php

class A {
  function f($a) {
    $a->f();
  }
}

function test() {
  $a = new A();
  $a->f(null);
}

test();

$ ./program -v Log.InjectedStackTrace=true
HipHop Fatal error: Unknown method stdClass::f
    #0 at [test_exc.php:5]
    facebook#1 A->f(), called at [test_exc.php:11]
    facebook#2 test(), called at [test_exc.php:14]

$ hphpi test_exc.php -v Log.InjectedStackTrace=true
HipHop Fatal error: Call to a member function f() on a non-object
    #0 at [/var/users/qixin/hphp2/src/tmp/test_exc.php:5]
    facebook#1 A->f(), called at [/var/users/qixin/hphp2/src/tmp/test_exc.php:11]
    facebook#2 test(), called at [/var/users/qixin/hphp2/src/tmp/test_exc.php:14]

DiffCamp Revision: 122030
Reviewed By: hzhao
CC: hphp-diffs@lists, hzhao, qixin
Tasks:
#220730: Support Log.InjectedStackTrace

Revert Plan:
OK
h4ck3rm1k3 pushed a commit to h4ck3rm1k3/hhvm that referenced this issue Sep 13, 2011
…l() coding

Summary:
So i'm correctly handling references and objects now. I'll need diff facebook#2-3 to
have globals, which really isn't different from params with all references and
objects. So, all marshaling unmarshaling code are done, just hooking up with
global states and thread locals.

Also fixed call_user_func() to respect references!

Test Plan:
the new unit tests

DiffCamp Revision: 122452
Reviewed By: iproctor
Commenters: qixin
CC: hphp-diffs@lists, iproctor, hzhao, qixin
Tasks:
#141384: add multi-threading support to PHP

Revert Plan:
OK
h4ck3rm1k3 pushed a commit to h4ck3rm1k3/hhvm that referenced this issue Sep 13, 2011
…Unmarshal() coding"

This reverts commit a36441d6f9849da2800e6af6a561cc31d9a3036e.
h4ck3rm1k3 pushed a commit to h4ck3rm1k3/hhvm that referenced this issue Sep 13, 2011
…Marshal/Unmarshal()

coding""

Summary:
This reverts commit c09b5963d6a8ed96928d94146764d072b650ff98.

Test Plan:
fast_tests, slow_tests, www

DiffCamp Revision: 123024
Reviewed By: qixin
CC: hphp-diffs@lists, hzhao, qixin
Tasks:
#141384: add multi-threading support to PHP

Revert Plan:
OK
h4ck3rm1k3 pushed a commit to h4ck3rm1k3/hhvm that referenced this issue Sep 13, 2011
Summary:
The mysql protocol allows the selection of an initial database on connect time.
We currently have to call mysql_select_db() immediately after establishing a
connection. This change will allow us to set the db at connect time, to avoid
unnecessary round trips.

Test Plan:
I created a local instance of mysql and these databases:
create database foo;
create database bar;
create table bar_table ( message varchar(64) );
insert into bar_table (message) values ('You selected from bar_table in bar
db!');
create table foo_table ( message varchar(64) );
insert into foo_table (message) values ('You selected from foo_table in foo
db!');

Then I ran this test code:

$c = mysql_connect('127.0.0.1:3306', 'root');
echo $c . "<br/>";
mysql_select_db('foo') ."<br/>";
print_r( mysql_fetch_array(mysql_query('show tables'), $c) );
echo "<br/>";
print_r( mysql_fetch_array(mysql_query('select * from foo_table'), $c) );
echo "<hr/>";

$d = mysql_connect_with_db('127.0.0.1:3306', 'root', '', "bar");
echo $d . "<br/>";
print_r( mysql_fetch_array(mysql_query('show tables'), $d) );
echo "<br/>";
print_r( mysql_fetch_array(mysql_query('select * from bar_table'), $d) );

Expected result:
Resource id facebook#1
Array ( [Tables_in_foo] => foo_table )
Array ( [message] => You selected from foo_table in foo db! )
Resource id facebook#2
Array ( [0] => bar_table )
Array ( [0] => You selected from bar_table in bar db! )

DiffCamp Revision: 138364
Reviewed By: macvicar
Commenters: ps, rmcelroy, mcallaghan
CC: hphp-diffs@lists, rmcelroy, ps, macvicar, mcallaghan, cbueno
Revert Plan:
OK
sgolemon pushed a commit that referenced this issue Mar 9, 2013
Move the responsibility of entering/exiting contexts from PHP to the
implementation of $wait_handle->join().

This eliminates possibility of weird situations, like contexts without
any running wait handle. This guarantees that asio_get_current() returns
null only if called completely out of asio framework and simplifies some
logic, such as getCurrentWaitHandleDepth().
sgolemon pushed a commit that referenced this issue Mar 22, 2013
Our PHP code no longer yields arrays. Remove.
facebook-github-bot pushed a commit that referenced this issue Nov 15, 2019
Summary:
In the smart constructor for unions Typing_union.make_union, if the result is a union which contains unsolved type variables, then we wrap it in a type variable.
Do something similar with `make_intersection`

When a type variable gets solved, unions containing this type variable may be solved in `Typing_type_simplifier` if all type variables from this union are solved.

This is because there are a lot of types like
```
( Vector<#1> |
  Vector<#2> |
  ...
  Vector<#n> )
```
If each time a type variable gets solved, we attempt a union simplification which is quadratic in n, we get bad perf.

Therefore, we only simplify a type when all type variables in it are solved.
To that end, we also maintain a map `tyvars_in_type` in the environment that is the dual of the other map `tyvar_occurrences` introduced in a previous diff. Furthermore, we now maintain the invariant that we only keep the occurrences links if the type variable that occurs is unsolved or itself contains unsolved type variables.

So for example, for
```
#1 -> (#2 | #3)
#2 -> (int | #4)
#4 -> string
#3 -> #5
#5 unsolved
```
We'd have recorded that #5 occurs in #3 and #3 in #1, but since #2 and #4 are solved and do not contain any unsolved type variables, we don't record their occurrences.

We can now simplify a type variable type only when their entry in `tyvars_in_type` is empty.

With the previous example, when #5 gets solved, if it gets solved to something that still contains unsolved type variable, we don't do any simplification, but if it gets solved to say int, to maintain the invariant described above, we'd remove it from the types in #3, and since #3 now wouldn't contain any more unsolved type variables we could simplify it, then recursively remove it from the types in #1 and finally simplify #1 itself.

We also remove now unnecessary usages of simplify_unions

Reviewed By: andrewjkennedy

Differential Revision: D18059114

fbshipit-source-id: 73eb8c40087d519246306382b9c2ab99ad1deb20
facebook-github-bot pushed a commit that referenced this issue Nov 22, 2019
Summary: In expand_type_and_solve, we detect whether we solve invariant type variables to nothing. The logic however is brittle. For example, if the type at hand is (#1 & nonnull) and #1 gets solved to nothing, it would fail to detect it, and therefore fail to fail. Same goes if the type at hand is (#1 | #2).

Reviewed By: andrewjkennedy

Differential Revision: D18635991

fbshipit-source-id: 33ce9d1b0630b1a8bf01ecdccf8d05363ce16072
facebook-github-bot pushed a commit that referenced this issue Nov 22, 2019
Summary:
Allows to clean up `push_option_out`.

This diff is the same as D17342028 which has been backed out by D18530295, so I'm putting it up for review again.
The reason it had been backed out was stack overflows on the hack codebase, due to a bad interaction with the subtyping case transforming
```
nonnull & T <: Thas_member("m", #1)
```
into
```
T <: ?#2
#2 <: Thas_member("m", #1)
```
Some tricky cases would keep producing new "#2" variables until we'd get a stack overflow.
The fix is D18573887, on which this diff depends, which introduces simple unions of constraint types to avoid creating those type variables.

Reviewed By: vassilmladenov

Differential Revision: D18594639

fbshipit-source-id: f40d9aa784b095054567ba63e3fb75063bac1a13
facebook-github-bot pushed a commit that referenced this issue Jan 20, 2020
Summary:
This diff changes splat (...) unpacking to use Tdestructure, and extends Tdestructure to have 3 main boxes for unpacking. There are two cases to explore:

# Tuple destructuring

Now, `Tdestructure` for splat (henceforth as "splat(...)") has three separate boxes for types. For example, the following example

```
function take(bool $b, float $f = 3.14, arraykey ...$aks): void {}
function f((bool, float, int, string) $tup): void {
  take(...$tup);
}
```
corresponds to the subtyping assertion
```
(bool, float, int, string) <: splat([#1], [opt#2], ...#3)
```
and results in
```
bool <: #1
float <: #2
arraykey <: #3
```

First, it is required that the tuple have enough elements to fill the required values -- in this case just `[#1]`. Then, if there are remaining elements, they are first used to fill the optional elements `[#2]`. Finally, the remainder are collected in the variadic element `#3`. If tuple elements remain but there is no variadic element, then we emit an error.

# Array destructuring

Previously, the typechecker would allow nonsense like
```
function f(int $i = 3): void {}
function g(int $i, int ...$j): void {}

function passes_typechecker(): void {
  f(...vec[1,2]); // throws
  f(...vec[]); // throws
}
```
Now, we require that splat operations with arrays have a variadic component to accept the values. We additionally ban unpacking into the required arguments of a function because the array could be empty. Unpacking into optionals is OK. Basically, we require that the function signature can accept the entire array.

**Note:** we still allow partial array destructuring with *list destructuring*, so
```
list($a, $b) = vec[1,2,3,4];
```
is still valid and is represented as
```
vec<int> <: list(#1, #2)
```

# Typing Rules

In these rules `r` is a type of a required field, `o` is a type of an optional field, `v` is the type of the variadic field.

#  destruct_tuple_list
```
forall i in 1...n

             t_i <: r_i
--------------------------------------
(t_1, ..., t_n) <: list(r_1, ..., r_n)
```
**Example**
```
function test_runner((int, string, bool) $tup): void {
  list($a, $b, $c) = $tup;
}
```

# destruct_tuple_splat
```
forall i in 1...m
forall j in (if p < n then m+1...p else m+1...n)
forall k in p+1...n

     m <= n    t_i <: r_i    t_j <: o_j    t_k <: v
-----------------------------------------------------------
(t_1, ..., t_n) <: splat(r_1, ..., r_m, o_m+1, ..., o_p, v)
```
**Example**
```
function take(int $i, string $j, bool $kopt = false, float $fopt = 3.14, mixed ...$rest): void {}

function test_runner((int, string) $tup): void { take(...$tup); }
function test_runner2((int, string, bool) $tup): void { take(...$tup); }
function test_runner3((int, string, bool, float) $tup): void { take(...$tup); }
function test_runner4((int, string, bool, float, null, int, mixed) $tup): void { take(...$tup); }
```

# destruct_array_list

This rule already exists, and it allows for operations that can throw.
```
      forall i in 1...n   t <: r_i
-------------------------------------------
        vec<t> <: list(r_1, ... r_n)
     Vector<t> <:
  ImmVector<t> <:
ConstVector<t> <:
```
**Example**
```
list($a, $b, $c) = vec[3]; // legal, but will throw
list($a) = vec[2,3,5]; // legal even though incomplete
```

# destructure_array_splat

Note the lack of required field in this row, and the presence of the variadic.

```
 forall i in 1..n   t <: o_i    t <: v
----------------------------------------
Traversable<t> <: splat(o_1, ... o_n, v)
```

**Example**
```
function take1(int $i = 3, int ...$is): void {}
function take2(int ...$is): void {}

function test(vec<int> $v): void {
   take1(...$v);
   take2(...$v);
}
```

Reviewed By: CatherineGasnier

Differential Revision: D18633271

fbshipit-source-id: 2b7a7beebf2ce30c2cb2765f75de2db6bdb3c24e
facebook-github-bot pushed a commit that referenced this issue Feb 13, 2020
Summary:
Pre-initialize all global variables before doing any merge operation.

Subgraphs (a list of (tyvar, constraint)-nodes) are not necessarily closed.

E.g. the two-node subgraph

```
~~~~~
    #2
   /
#1
......
#1
   \
    #4
~~~~~
```

will:
   1. Copy #1 into the merge-env
   2. Try to close #4 with #2, but they won't be initialized the environment because they will be declared in subsequent subgraphs.

So we have to do the initialization up front, for all tyvars, at the same time

Reviewed By: CatherineGasnier

Differential Revision: D19813054

fbshipit-source-id: 7e51b700f3d508c2ef2075c3b6d5db0e5c500ded
facebook-github-bot pushed a commit that referenced this issue Feb 29, 2020
…he same connected component

Differential Revision: D20143605

fbshipit-source-id: 3ede5be308367ef31da6378261e86cbe36fc8c38
facebook-github-bot pushed a commit that referenced this issue Apr 6, 2020
Summary:
I've been briefly looking at usage of legacy arrays inside runtime after last HAM meeting, and I decided to clean some of those up.

This particular change proved a lot more exciting than I thought at first, as naively converting legacy arrays to varray/darray exposed lots of surprises

- some of ini parsing code (`IniSettingMap`, `IniSetting::FromStringAsMap()`) is used during hhvm options parsing and can not use v/darrays, because v/darrays can not exist inside runtime when Eval.HackArrDVArrs=true.
- Rust options parser consumes HHVM options via json and really wants list-like options to be encoded as json lists
- ini parsing is also exposed to userspace via `parse_ini_string()`/`parse_ini_file()` builtins).
- ini values can be read with ini_get() / ini_get_all()

I ended up solving these issues by
1. convert parts of ini parsing, which are used during early hhvm startup, to use dicts.
2. convert parts of ini parsing, which are exposed as `parse_ini_string()` to use darrays
3. generalize ini parser callbacks to support both #1 and #2
4. make ini_get() functions produce either darray or varray depending on corresponding c++ type. These functions are used for ini_get/ini_get_all builtins

The resulting change is a bit ugly, but we'll need to make similar changes anyway, when we decide to kill legacy array completely, so I'd rather land this now.

In terms of observable behavior, this diff results in the following:
- ini_get / ini_get_all returns varrays & darrays (depending on C++ data type for an underlying option)
- parse_ini_string / parse_ini_file always returns darrays
- ini parsing works with both Eval.HackArrDVArrs on and off
- ini parsing no longer uses legacy php arrays

Differential Revision: D20805919

fbshipit-source-id: 50e72ef979ef2618ec5d39a9ed0a3f35bf330cc2
facebook-github-bot pushed a commit that referenced this issue May 1, 2020
Summary:
This is a redo of D20805919 with a fix for the crash, described in T65255134.

Crash stack:

```
#0  0x0000000006566dbe in HPHP::arrprov::tagFromPC () at hphp/runtime/base/array-provenance.cpp:343
#1  0x000000000670b2d5 in HPHP::arrprov::tagStaticArr (tag=..., ad=<optimized out>) at hphp/runtime/base/array-provenance.cpp:291
#2  HPHP::ArrayData::CreateDict (tag=..., tag=...) at buck-out/opt-hhvm-lto/gen/hphp/runtime/headers#header-mode-symlink-tree-only,headers,v422239b/hphp/runtime/base/array-data-inl.h:105
#3  HPHP::Array::CreateDict () at buck-out/opt-hhvm-lto/gen/hphp/runtime/headers#header-mode-symlink-tree-only,headers,v422239b/hphp/runtime/base/type-array.h:97
#4  HPHP::IniSettingMap::IniSettingMap (this=<optimized out>, this=<optimized out>) at hphp/runtime/base/ini-setting.cpp:517
#5  0x00000000068835bf in HPHP::IniSettingMap::IniSettingMap (this=0x7fff36f4d8f0) at hphp/runtime/base/config.cpp:370
#6  HPHP::Config::Iterate(std::function<void (HPHP::IniSettingMap const&, HPHP::Hdf const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>, HPHP::IniSettingMap const&, HPHP::Hdf const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) (cb=..., ini=..., config=..., name=..., prepend_hhvm=prepend_hhvm@entry=true) at hphp/runtime/base/config.cpp:370
#7  0x0000000006927373 in HPHP::RuntimeOption::Load (ini=..., config=..., iniClis=..., hdfClis=..., messages=messages@entry=0x7fff36f4e170, cmd=...)
    at third-party-buck/platform007/build/libgcc/include/c++/trunk/bits/std_function.h:106
#8  0x000000000695aaee in HPHP::execute_program_impl (argc=argc@entry=21, argv=argv@entry=0x7fff36f4fac8) at hphp/runtime/base/program-functions.cpp:1716
```

The immediate cause of the crash is trying to access vmfp() inside tagFromPC before VM was initialized, resulting in null pointer dereference.
However, provenance tagging should have actually used runtime tag override, set in RuntimeOption::Load instead of trying to compute tag off PC.
The problem is: TagOverride short-circuits itself if Eval.ArrayProvenance is disabled.

So we run into the following sequence of events:
1. we start with EvalArrayProvenance=false - default value
2. TagOverride short-circuits and doesn't actually update the override
3. we parse config options and set EvalArrayProvenance=true
4. we try to create dict, decide that it needs provenance and try to compute a tag. Since there is no override set we fall back to tagFromPC and crash

To fix this I made TagOverride not short-circuit for this 1 specific call site.

The specific nature of a bug also explains, why it didn't get caught by any of prior testing:
- hphp tests could not catch it, because they run with minimal config, which doesn't exercise Config::Iterate
- I didn't specifically test sandbox with array provenance enabled, which would catch it
- This wouldn't be caught in servicelab, since we enable provenance in SL by changing default values. Also, I didn't run SL with provenance enabled for this.

What would catch this is starting either sandbox or prod-like web server with actual config.hdf or config.hdf.devrs and array provenance enabled via config or commandline options.

Differential Revision: D20974470

fbshipit-source-id: 6474fe4e4cf808c4e8572539119cd57374658877
facebook-github-bot pushed a commit that referenced this issue May 2, 2020
Summary:
After carefully chasing the code, finally I understand how dump_config works,

1. same config is printed for each file if it runs in multiple file mode,
2. no config is printed if output file is specified.

#2 makes user confusing why no config printed if both --dump-config and output file are specified.

a lazy evaluation is introduced to avoid repeated computation in multiple file mode.

This diff simplifies --dump-config,
1. always print to stdout if --dump-config,
2. only print once to stdout if --dump-config. Therefore, lazy evaluation can be removed.

Reviewed By: losvald

Differential Revision: D21346578

fbshipit-source-id: 085aa00590a03b76ff4b598c29f74bf94d688ae0
facebook-github-bot pushed a commit that referenced this issue Jul 14, 2020
Summary:
I was trying to run servicelab with provenance enabled and had run into HHBC crash. Stack trace:

```
#0  0x0000000005e2b553 in HPHP::arrprov::tagFromPC () at buck-out/opt-hhvm-lto/gen/hphp/runtime/headers#header-mode-symlink-tree-only,headers,v5c9e8e3/hphp/runtime/base/rds-header.h:127
#1  0x0000000005e2f059 in HPHP::arrprov::tagStaticArr (ad=<optimized out>, tag=...) at hphp/runtime/base/array-provenance.cpp:297
#2  0x00000000007b8b5b in HPHP::ArrayData::CreateDArray (tag=..., tag=...) at buck-out/opt-hhvm-lto/gen/hphp/runtime/headers#header-mode-symlink-tree-only,headers,v5c9e8e3/hphp/runtime/base/array-data-inl.h:60
#3  HPHP::Array::CreateDArray () at buck-out/opt-hhvm-lto/gen/hphp/runtime/headers#header-mode-symlink-tree-only,headers,v5c9e8e3/hphp/runtime/base/type-array.h:109
#4  HPHP::preg_match_impl (pattern=pattern@entry=0x7f408a805860, subject=subject@entry=0x7f408a805840, subpats=subpats@entry=0x0, flags=flags@entry=0, start_offset=start_offset@entry=0, global=false) at hphp/runtime/base/preg.cpp:1146
#5  0x00000000060bdf89 in HPHP::preg_match (offset=0, flags=0, matches=0x0, subject=<optimized out>, pattern=0x7f408a805860) at buck-out/opt-hhvm-lto/gen/hphp/runtime/headers#header-mode-symlink-tree-only,headers,v5c9e8e3/hphp/runtime/base/memory-manager-inl.h:114
#6  HPHP::preg_match (flags=0, offset=0, matches=0x0, subject=..., pattern=...) at hphp/runtime/base/preg.cpp:1382
#7  HPHP::Config::matchHdfPattern (value=..., ini=..., hdfPattern=..., name=..., suffix=...) at hphp/runtime/base/config.cpp:395
#8  0x000000000885be96 in HPHP::(anonymous namespace)::applyBuildOverrides (ini=..., config=...) at hphp/util/hdf.cpp:90
#9  0x000000000886f000 in HPHP::prepareOptions (argv=<optimized out>, argc=51, po=...) at hphp/compiler/compiler.cpp:450
#10 HPHP::compiler_main (argc=51, argv=<optimized out>) at hphp/compiler/compiler.cpp:157
#11 0x00007f4120b001a6 in __libc_start_main (main=0x1cf2080 <main(int, char**)>, argc=52, argv=0x7ffda8b93048, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffda8b93038) at ../csu/libc-start.c:308
#12 0x000000000b31cb5a in _start () at ../sysdeps/x86_64/start.S:120
```

Differential Revision: D22519863

fbshipit-source-id: 2ce590181d7f7259e490ce3206c08e76f4eaaa4b
facebook-github-bot pushed a commit that referenced this issue Nov 2, 2020
Summary: My goal is to get a handle on telemetry about which decls are read, when, and why. A significant number of costly decls are read during variance-checking. To get higher-value telemetry, I want to know which filename we're currently checking at the time we fetch decls. That's what this diff achieves.

Reviewed By: CatherineGasnier

Differential Revision: D24585857

fbshipit-source-id: 624e596fcda09c60c3a06bb459925d21e3a77ecf
facebook-github-bot pushed a commit that referenced this issue Apr 21, 2021
Summary:
Update the heuristic for reporting errors when solving a type leads to type `nothing`. Previously, if there were any unsolved type variables, and the type solved to `nothing` an error was generated suggesting a type to  add to the program. The assumption was that the resulting `nothing` was as a result of the type variable itself solving to `nothing`. However, the type could have solved to `nothing` for other reasons that have nothing to do with the variable (e.g., it contained a variable that solved to `null` intersected with one that solved to `nonnull`). This makes the heuristic more precise, in that it will only produce the error when a type variable is actually solved to `nothing` in the process of the whole type solving to `nothing`. This extra precision makes the type checker more complete.

The updated heuristic can change which suggestion gets made. Consider the situation where the type is a variable #1, and we also had #2<:#1. Previously, #1 would get solved to #2 which would get solved to `nothing`, and the error would be based on #1 from the original type. Now, since it's #2 that gets solved to `nothing` the error is based on it instead.

Reviewed By: CatherineGasnier

Differential Revision: D27879773

fbshipit-source-id: 4e86851efb7b54611e9dfa1c444bde13750982f6
facebook-github-bot pushed a commit that referenced this issue May 11, 2021
Summary:
This diff addresses Monitor_connection_failure. It's all related to the question of how we behave when we have a huge number of incoming requests. The end result is (1) no more Monitor_connection_failure ever, (2) hh will wait indefinitely, or up to --timeout, for a connection to the server.
```
[client] * N -> [monitor] -> [server]
```
1. The server has a finite incoming monitor->server pipe, and it runs an infinite loop: (1) pick the next workitem of the incoming pipe, (2) that frees up a slot in the pipe so the monitor will be able to write the next item into it, (3) do a typecheck if needed, (4) handle the workitem
2. The monitor has a socket with a finite incoming queue, and runs an infinite loop: (1) pick the next workitem off the incoming queue, (2) that frees up a slot in the queue so the next client will be able to write into it, (3) attempt to write the workitem on the monitor->server pipe, but if that queue has remained full for 4s then communicate back to the workitem's client that it was busy.
3. Each client invocation attempts to put a workitem on the monitor's queue. It has a timeout of 1s. If the monitor's queue was full, it fails fatally with Monitor_connection_failed. If the monitor failed to handoff then it fails with Server_hung_up_should_retry and retries exponentially.

## Callers of hh_client should decide timeouts.

There's a "hh_client --timeout" parameter (by default infinite). The right design is that if the server is too busy, we should wait until it's available, up to that timeout. It's not right for us to unilaterally fatal when the server is busy. That's what this diff achieves. This means we no longer have a finite queue of hh_client requests (manifesting as Monitor_connection_failure roughly when it gets too big); it's now an infinite queue, embodied in the unix process list, as a set of processes all waiting for their chance.

> Note: HackAst generally doesn't specify a timeout. (1) If arc lint kicks off too many requests, that used to manifest as some of them getting failures but hh_server becomes available after 30-60s as they all fail; now it'll just go in a big queue, and the user will see in the spinner that hh_server is busy with a linter. (2) If an flib test kicks off too many requests, that used to manifest as some of them getting Monitor_connection_failure failures; now it'll manifest as the whole flib test timing out. (3) If a codemod kicks off too many requests, that used to manifest as the codemod failing with Monitor_connection_failure; now it'll manifest as the codemod taking longer but succeeding.

## It's all about backpressure.

Imagine that the system is full to capacity with short requests. Backpressure from the server is indicated by it only removing items from the monitor->server pipe as the server handles requests. This backpressure goes to the monitor, which can only handoff to the server once per server-handling, and hence can only pull items off its own incoming queue at that time too. This backpressure goes to the clients, which will timeout if they can't put an item on the monitor's queue. To make backpressure flow correctly, this diffstack makes the following changes:
1. The monitor will wait up to 30s for the server to accept a workitem. All normal server requests will be handled in this time. The ones that won't are slow find-all-refs and slow typechecks. (The previous 4s meant that even normal requests got the user-facing message "server isn't responding; reconnecting...")
2. The client will have a timeout up to "hh_client --timeout" in its attempts to put work onto the monitor's incoming queue. If the user didn't specify --timeout (i.e. infinite) then I put a client timeout of 60s plus automatic immediate retries. (The previous Monitor_connection_failure fatal error was the whole problem).

## We never want the server to go idle.

The best way for the monitor to wait is *as part of* its attempt to handoff to the server. This way, as soon as the server is ready, it will have another workitem and will never go idle. Likewise, the best way for a client to wait is *as part of* its attempt to establish a connection to the monitor and put a workitem on the monitor's incoming queue. That way, as soon as the monitor is ready, it will have another workitem and will never go idle. We should *not* be waiting in `Unix.sleep`; that's always wrong.  Effectively, our clients will park themselves in the unix process list, and the kernel will know that they're waiting for a slot to become available in the monitor's incoming queue, and will wake them up when it becomes available. This is a better than the current solution of cancelling and retrying ever 1s, a kind of "busy spin loop" which pointlessly burns CPU.

## Why is the monitor-to-server handoff timeout 30s?

For the handoff timeout, why did I put 30s rather than infinite?

There is only one scenario where we will exceed the 30s timeout. That's when both (1) the server's queue is full because there's been a high rate of incoming requests, (2) the server's current work, either for a file-change-triggered typecheck or a costly client request, takes longer than 30s.

*This scenario is a misuse of hack*. It simply doesn't make sense to have a high volume of costly requests. The computer will never be able to keep up. It's fine to have a high volume of cheap requests e.g. from typical Aurora linters. It's fine to have a low volume of expensive requests e.g. from a file change, or --find-refs. But the combination should be warned about. When the handoff times out, the monitor will send Monitor_failed_to_handoff to the client, which will display to the user "Hack server is too busy", and fail with exit code Server_hung_up_should_retry, and find_hh.sh will display "Reconnecting..." and retry with exponential backoff. That's not the mechanism I'd have chosen, but it's at least a reasonable mechanism to alert to the user that something's seriously wrong but still without failing.

If I'd used an infinite timeout then the user would never see "Hack is too busy. Reconnecting...". Also I have a general distrust of infinite timeouts. I worry that there will be cases where something got stuck, and I'd like for the user to at least become unstuck after a minute. If I'd used a smaller timeout like the current 4s, then the user will see this "Reconnecting..." message even in reasonable high-request-rate scenarios like --type-at-pos-batch. 30s feels like a good cutoff. It will display a message in the CLI if hack is being misused, as I described.

## Why is the client timeout 60s?

In the (default) case where the user didn't specify a --timeout parameter and we therefore use infinite timeout, I again got cold feet at putting an infinite timeout. I instead set it at 60s plus immediate retry if that times out. This is a bit like a busy spin loop, but it's not terribly busy. I got cold feet because there might be unknown other causes of failure and I didn't want them to just hang indefinitely.

Let's examine what the client code actually does for the "60s/retry" case:
1. Server_died. This happens from ServerMonitor sending Prehandoff.Server_died or Prehandoff.Server_died_config_changed. (I think there are bugs here; if the server died then the current hh_client should exit with code 6 "server_hung_up_should_retry" so that find_hh.sh will launch a new version of the hh_client binary. But I'll leave that for now). In this case hh_client retries
2. Timeout for the monitor to accept the request. This means there is backpressure from the monitor telling clients to slow down. This is the case where hh_client used to fatally exit with Monitor_connection_failure exit code 9. Now, with this diff, it hits the 60s timeout then immediately retries.
3. Timeout AFTER the monitor has accepted the request but before it's handed off. In this case hh_client has always retried
4. Timeout while the monitor attempts to handoff. This is the "30s monitor handoff timeout" discussed above. In this case hh_client exits with exit code 6, and find_hh.sh retries with exponential backoff, and the user sees "Server has hung up. Reconnecting..."
5. We don't even measure timeout while waiting for the server to process the request.

In the past (see comments in previous diff), if case 2 just does an immediate retry, then the test plan gets into a problem state where the monitor is just stuck trying to read client version. Previously it had been described as "wedged". I think that was caused by clients having a low timeout of just 1s, smaller indeed than the monitor->server timeout. But since I changed the client timeout to 60s that no longer happens.

*30s < 60s to avoid livelock pathology.*

The client's timeout must be comfortably longer than the monitor's timeout. Imagine if it were reversed, and the client's timeout was 30s while the monitor's was 60s. Therefore ever workitem on the monitor's queue would be stale by the time the monitor picks it up, and it won't get anything to send to the server, and after it burns through its queue rejecting every one (since each client is no longer around) then it'll have to just wait until a client retries. That would be a self-inflicted busy loop.

Let's spell it out. It had been observed in June 2017 that if there were 1000 parallel clients then the monitor became "wedged". It didn't define precisely what was observed, but D5205759 (b57d70b) explained a hypothesis:
1. Imagine the monitor's incoming socket queue is size 1000, and there are 1000 parallel clients.
2. Imagine that the first 999 of these clients has already given up (due to timeout), and so as the monitor works through them, for each one when it attempts to send Connection_ok back to that client, it will get EPIPE. Imagine it takes the monitor 1ms for this.
3. Clients in general have a 1s timeout which encompasses both waiting until the monitor enqueues it, and waiting until the monitor responds to it. If this deadline expires then they wait 0.1s and retry.
4. Imagine our client was number 1000, but because it took 999ms already to get EPIPE back from the earlier items, then by the time the monitor is ready to deal with us, then we've probably expired our 1s time limit, so the monitor will cancel us too.
5. In this time 999 other clients have again added themselves to the queue. We too will add ourselves to the queue. And the cycle will repeat again.

I experimented prior to this diffstack but was unable to reproduce this behavior. What I instead observed was that the client was in an ongoing loop of trying to connect to the monitor but failing because the monitor's incoming queue was full, and the monitor had failed to pick up anything from its queue for over five minutes. Here's what the monitor log and pstack looked like:
```
[2021-05-04 15:31:28.825] [t#O8Dg1HTCkS] read_version: got version {"client_version":"","tracker_id":"t#O8Dg1HTCkS"}, started at [2021-05-04 15:31:28.825]
[2021-05-04 15:31:28.825] [t#O8Dg1HTCkS] sending Connection_ok...
[2021-05-04 15:31:28.825] SIGPIPE(-8)
[2021-05-04 15:31:28.825] Ack_and_handoff failure; closing client FD: Unix.Unix_error(Unix.EPIPE, "write", "")
[2021-05-04 15:31:28.825] [t#oKIHi8xMAS] read_version: got version {"client_version":"","tracker_id":"t#oKIHi8xMAS"}, started at [2021-05-04 15:31:28.825]
[2021-05-04 15:31:28.825] [t#oKIHi8xMAS] sending Connection_ok...
[2021-05-04 15:31:28.825] SIGPIPE(-8)
[2021-05-04 15:31:28.825] Ack_and_handoff failure; closing client FD: Unix.Unix_error(Unix.EPIPE, "write", "")

#1  0x00000000025d0b31 in unix_select (readfds=<optimized out>, writefds=<optimized out>, exceptfds=<optimized out>, timeout=113136840) at select.c:94
#2  0x00000000022ef1f4 in camlSys_utils__select_non_intr_8811 () at /data/users/ljw/fbsource/fbcode/hphp/hack/src/utils/sys/sys_utils.ml:592
#3  0x00000000022a2666 in camlMarshal_tools__read_217 () at /data/users/ljw/fbsource/fbcode/hphp/hack/src/utils/marshal_tools/marshal_tools.ml:136
#4  0x00000000022a302b in camlMarshal_tools__from_fd_with_preamble_383 () at /data/users/ljw/fbsource/fbcode/hphp/hack/src/utils/marshal_tools/marshal_tools.ml:263
#5  0x0000000001f90116 in camlServerMonitor__read_version_2818 () at /data/users/ljw/fbsource/fbcode/hphp/hack/src/monitor/serverMonitor.ml:315
#6  0x0000000001f9106a in camlServerMonitor__ack_and_handoff_client_4701 () at /data/users/ljw/fbsource/fbcode/hphp/hack/src/monitor/serverMonitor.ml:548
#7  0x0000000001f92509 in camlServerMonitor__check_and_run_loop__4776 () at /data/users/ljw/fbsource/fbcode/hphp/hack/src/monitor/serverMonitor.ml:842
#8  0x0000000001f920cc in camlServerMonitor__check_and_run_loop_inner_5965 () at /data/users/ljw/fbsource/fbcode/hphp/hack/src/monitor/serverMonitor.ml:779
```
The client log is monstrously huge and full, for the final five minutes, of repeated (and failed-due-to-timeout) attempts to open the socket to the monitor:
```
[2021-05-04 15:53:00.223] [Check#IRhKGkxQn7] [t#QpQxPnW9wt] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.223] [Check#0NzdVomX9R] [t#6QT1JEbgJK] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.223] [Check#I9hYXmPzb0] [t#o9PRU7m1lM] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.224] [Check#CLtBMgCEVG] [t#WwlyQwUDhn] [client-connect] ClientConnect.connect: attempting MonitorConnection.connect_once
[2021-05-04 15:53:00.224] [Check#CLtBMgCEVG] [t#WwlyQwUDhn] Connection_tracker.Client_start_connect
[2021-05-04 15:53:00.224] [Check#zXvag0PQCK] [t#2NOmFISCOc] [client-connect] ClientConnect.connect: attempting MonitorConnection.connect_once
[2021-05-04 15:53:00.224] [Check#YoJPxiGVjK] [t#CICotuALSY] [client-connect] ClientConnect.connect: attempting MonitorConnection.connect_once
[2021-05-04 15:53:00.224] [Check#jCTjq2AcPp] [t#HvO1E3Rv10] [client-connect] ClientConnect.connect: attempting MonitorConnection.connect_once
[2021-05-04 15:53:00.224] [Check#VShC3XccG0] [t#LLbhdQhcch] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.224] [Check#YoJPxiGVjK] [t#CICotuALSY] Connection_tracker.Client_start_connect
[2021-05-04 15:53:00.224] [Check#MhnHnQmHIj] [t#u3gm7WR4B3] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.224] [Check#jCTjq2AcPp] [t#HvO1E3Rv10] Connection_tracker.Client_start_connect
[2021-05-04 15:53:00.224] [Check#U72xNuSb7N] [t#S47cx27Lta] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.224] [Check#zXvag0PQCK] [t#2NOmFISCOc] Connection_tracker.Client_start_connect
[2021-05-04 15:53:00.224] [Check#eGHledjDNT] [t#bQPqB2iG6c] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.224] [Check#7xj4rdPkhA] [t#NKGlmJYKAC] [client-connect] ClientConnect.connect: attempting MonitorConnection.connect_once
[2021-05-04 15:53:00.225] [Check#7xj4rdPkhA] [t#NKGlmJYKAC] Connection_tracker.Client_start_connect
[2021-05-04 15:53:00.225] [Check#kxQ7dr1Ve2] [t#JKFk5Pl5DB] [client-connect] ClientConnect.connect: attempting MonitorConnection.connect_once
[2021-05-04 15:53:00.225] [Check#kxQ7dr1Ve2] [t#JKFk5Pl5DB] Connection_tracker.Client_start_connect
[2021-05-04 15:53:00.225] [Check#s5HbtmgdVN] [t#ywpyouI5po] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.225] [Check#5192H11uaP] [t#ku2JO0LNvX] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
[2021-05-04 15:53:00.225] [Check#yOV3DR3w9] [t#r1irfsdLn5] CLIENT_CONNECT_ONCE_FAILURE {"kind":"Connection_to_monitor_Failure","server_exists":true,"phase":"ServerMonitorUtils.Connect_open_socket","reason":"timeout","exn":null,"exn_stack":null}
```

Differential Revision: D28205345

fbshipit-source-id: a0f011ff2bcb379c3186ae5e319c4d1e9912c988
facebook-github-bot pushed a commit that referenced this issue May 13, 2021
Summary:
The original/'fixed' version of this regex put the `\d+` inside a character class, so the `+` never worked correctly for double-digit counts, resulting in errors like this.

```
> hphp/test/run --retranslate-all 5

Fatal error: Uncaught exception 'HH\InvariantException' with message 'unexpected output from shell_exec in runif_test_for_feature: 'Notice: File could not be loaded: 0'' in /data/users/wkl/fbsource/fbcode/hphp/test/run.php:2133
Stack trace:
#0 /data/users/wkl/fbsource/fbcode/hphp/test/run.php(2133): HH\invariant_violation()
#1 /data/users/wkl/fbsource/fbcode/hphp/test/run.php(2189): runif_test_for_feature()
#2 /data/users/wkl/fbsource/fbcode/hphp/test/run.php(2326): runif_function_matches()
#3 /data/users/wkl/fbsource/fbcode/hphp/test/run.php(2965): runif_should_skip_test()
#4 /data/users/wkl/fbsource/fbcode/hphp/test/run.php(2935): run_test()
#5 /data/users/wkl/fbsource/fbcode/hphp/test/run.php(1988): run_and_log_test()
#6 /data/users/wkl/fbsource/fbcode/hphp/test/run.php(3770): child_main()
#7 /data/users/wkl/fbsource/fbcode/hphp/test/run.php(3897): main()
#8 (): run_main()
#9 {main}
```

how_to_regex_garabatokid

Reviewed By: ottoni, mofarrell

Differential Revision: D28401997

fbshipit-source-id: 54dff6a21612cdeea53ddc31e99f4e41fd8205c9
facebook-github-bot pushed a commit that referenced this issue Jul 8, 2021
Summary:
My test runs are failing with errors like this: https://www.internalfb.com/intern/testinfra/diagnostics/5629499593789230.281474979307742.1625688677/

Text:

```
Failed to list tests.
STDOUT:{"args":"-m interp,jit","name":"hphp/test/server/debugger/tests/runTest1.php"}

STDERR:
Fatal error: Uncaught exception 'InvalidOperationException' with message 'Implicit null to string conversion for string concatenation/interpolation' in /data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/fbcode/hphp/test/run.php:195
Stack trace:
#0 /data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/fbcode/hphp/test/run.php(3661): success()
#1 /data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/fbcode/hphp/test/run.php(3942): main()
#2 (): run_main()
#3 {main}
```

Following the stacktrace, we see that the error occurs here:

https://www.internalfb.com/code/fbsource/[4981de7c3ed6cbbcd309a95da5b0c924e7c00b7e][history]/fbcode/hphp/test/run.php?lines=194-197

One frame up:

https://www.internalfb.com/code/fbsource/[4981de7c3ed6cbbcd309a95da5b0c924e7c00b7e][history]/fbcode/hphp/test/run.php?lines=3661

And `list_tests()` is a void function:

https://www.internalfb.com/code/fbsource/[4981de7c3ed6cbbcd309a95da5b0c924e7c00b7e][history]/fbcode/hphp/test/run.php?lines=778-795

So if we can't insert nulls into strings, this is bound to fail every time.

Reviewed By: DavidSnider

Differential Revision: D29597205

fbshipit-source-id: 852ffccf2f337b6dc1cb0909b3a276d97eced04f
facebook-github-bot pushed a commit that referenced this issue Jul 18, 2022
Summary:
Environment looping for loops was buggy in a very interesting way. This diff fixes it.

Consider the following fragment where the first dictionary is allocated at position `p` and the second one at `q`.
```
$d = dict['p' => 42];
while(...) {
  $d = dict['q' => true];
}
```

We were generating the constraints,
```
p -> #1 // line 1
q -> #2 // line 3
#1 \/ #2 -> #3 // line 4 on exit
#2 -> #1 // line 2-4
```
The last constraint is added because the definition of `$d` at the end of the loop needs to be related to the beginning of the loop.

However, this implies the following
```
p -> #1
q -> #1
q -> #2
```

The way we compute optional fields is if the join point has one entity in one branch but not in the other. So the solver concluded that `p` was an optional field but `q` was not (despite the fact that it is optional due to the fact that the loop body might never run)!

The problem is that we were using the same environment for at the beginning of the loop body and right before it. The solution is to simply create a redirection when we start typechecking the loop body and loop back onto that. This leads to the constraints:

```
p -> #1 // line 1
#1 -> #2 // due to refresh
q -> #3 // line 3
#1 \/ #3 -> #4 // line 4 on exit
#3 -> #2 // line 2-4
```

Then the solver concludes
```
p -> #1
q -> #3
q -> #2
```

Consequently, the solver concludes that at the join point `#4` both `p` and `q` are optional as one doesn't exist in `#1` and the other doesn't exist in `#3`.

Differential Revision: D37689958

fbshipit-source-id: 66cc8242fd87d99b5c495ff2747d60ba04a60a5a
facebook-github-bot pushed a commit that referenced this issue Oct 7, 2022
Summary:
We have seen deadlock running `terminationHandler` -> `hasSubscribers` in 2 threads.
It's unclear which other thread is holding the lock.

To make things easier to debug next time, let's change terminationHandler (and
also main.cpp) to bypass the logging lock and write to stderr directly.

Related threads (all threads in P536343453):

  Thread 11 (LWP 3275661):
  #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
  #1  0x0000000001cc995b in folly::detail::(anonymous namespace)::nativeFutexWaitImpl (addr=<optimized out>, expected=<optimized out>, absSystemTime=<optimized out>, absSteadyTime=<optimized out>, waitMask=<optimized out>) at fbcode/folly/detail/Futex.cpp:126
  #2  folly::detail::futexWaitImpl (futex=0x89, futex@entry=0x7f1c3ac2ef90, expected=994748889, absSystemTime=absSystemTime@entry=0x0, absSteadyTime=<optimized out>, absSteadyTime@entry=0x0, waitMask=waitMask@entry=1) at fbcode/folly/detail/Futex.cpp:254
  #3  0x0000000001d34bce in folly::detail::futexWait<std::atomic<unsigned int> > (futex=0x7f1c3ac2ef90, expected=137, waitMask=1) at buck-out/v2/gen/fbcode/110b607930331a92/folly/detail/__futex__/headers/folly/detail/Futex-inl.h:96
  #4  folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever::doWait (this=<optimized out>, futex=..., expected=137, waitMask=1) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:718
  #5  folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::futexWaitForZeroBits<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, state=@0x7f1c149f88e4: 118379409, goal=128, waitMask=1, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1184
  #6  0x0000000001cd42b2 in folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::yieldWaitForZeroBits<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, state=@0x7f1c149f88e4: 118379409, goal=128, waitMask=1, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1151
  #7  folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::waitForZeroBits<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, state=@0x7f1c149f88e4: 118379409, goal=128, waitMask=1, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1109
  #8  0x0000000001e7e14c in folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::lockSharedImpl<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, state=@0x7f1c149f88e4: 118379409, token=0x0, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1664
  #9  folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::lockSharedImpl<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, token=0x0, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1356
  #10 folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::lock_shared (this=0x7f1c3ac2ef90) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:495
  #11 std::shared_lock<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> >::shared_lock (this=<optimized out>, __m=...) at fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/shared_mutex:727
  #12 0x0000000002d765fd in folly::LockedPtr<folly::Synchronized<watchman::Publisher::state, folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> > const, folly::detail::SynchronizedLockPolicy<(folly::detail::SynchronizedMutexLevel)2, (folly::detail::SynchronizedMutexMethod)0> >::doLock<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>, std::shared_lock<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> >, folly::detail::SynchronizedLockPolicy<(folly::detail::SynchronizedMutexLevel)2, (folly::detail::SynchronizedMutexMethod)0>, 0> (mutex=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__synchronized__/headers/folly/Synchronized.h:1493
  #13 folly::LockedPtr<folly::Synchronized<watchman::Publisher::state, folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> > const, folly::detail::SynchronizedLockPolicy<(folly::detail::SynchronizedMutexLevel)2, (folly::detail::SynchronizedMutexMethod)0> >::LockedPtr (this=0x7f1c149f8928, parent=<optimized out>) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__synchronized__/headers/folly/Synchronized.h:1272
  #14 folly::SynchronizedBase<folly::Synchronized<watchman::Publisher::state, folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> >, (folly::detail::SynchronizedMutexLevel)2>::rlock (this=<optimized out>) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__synchronized__/headers/folly/Synchronized.h:229
  #15 watchman::Publisher::hasSubscribers (this=<optimized out>) at fbcode/watchman/PubSub.cpp:117
  #16 0x0000000002eca798 in watchman::Log::log<char const (&) [39], char const*, char const (&) [3]> (this=<optimized out>, level=level@entry=watchman::ABORT, args=..., args=..., args=...) at buck-out/v2/gen/fbcode/110b607930331a92/watchman/__logging__/headers/watchman/Logging.h:42
  #17 0x0000000002ec9ba7 in watchman::log<char const (&) [39], char const*, char const (&) [3]> (level=watchman::ABORT, args=..., args=..., args=...) at buck-out/v2/gen/fbcode/110b607930331a92/watchman/__logging__/headers/watchman/Logging.h:121
  #18 (anonymous namespace)::terminationHandler () at fbcode/watchman/SignalHandler.cpp:159
  #19 0x00007f1c3b0c7b3a in __cxxabiv1::__terminate (handler=<optimized out>) at ../../.././libstdc++-v3/libsupc++/eh_terminate.cc:48
  #20 0x00007f1c3b0c7ba5 in std::terminate () at ../../.././libstdc++-v3/libsupc++/eh_terminate.cc:58
  #21 0x0000000001c38c8b in __clang_call_terminate ()
  #22 0x0000000003284c9e in folly::detail::terminate_with_<std::runtime_error, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&> (args=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/lang/__exception__/headers/folly/lang/Exception.h:93
  #23 0x0000000003281bae in folly::terminate_with<std::runtime_error, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&> (args=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/lang/__exception__/headers/folly/lang/Exception.h:123
  #24 folly::SingletonVault::fireShutdownTimer (this=<optimized out>) at fbcode/folly/Singleton.cpp:499
  #25 0x0000000003281ad9 in folly::(anonymous namespace)::fireShutdownSignalHelper (sigval=...) at fbcode/folly/Singleton.cpp:454
  #26 0x00007f1c3b42b939 in timer_sigev_thread (arg=<optimized out>) at ../sysdeps/unix/sysv/linux/timer_routines.c:55
  #27 0x00007f1c3b41fc0f in start_thread (arg=<optimized out>) at pthread_create.c:434
  #28 0x00007f1c3b4b21dc in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

  ...

  Thread 1 (LWP 3201992):
  #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
  #1  0x0000000001cc995b in folly::detail::(anonymous namespace)::nativeFutexWaitImpl (addr=<optimized out>, expected=<optimized out>, absSystemTime=<optimized out>, absSteadyTime=<optimized out>, waitMask=<optimized out>) at fbcode/folly/detail/Futex.cpp:126
  #2  folly::detail::futexWaitImpl (futex=0x89, futex@entry=0x7f1c3ac2ef90, expected=994748889, absSystemTime=absSystemTime@entry=0x0, absSteadyTime=<optimized out>, absSteadyTime@entry=0x0, waitMask=waitMask@entry=1) at fbcode/folly/detail/Futex.cpp:254
  #3  0x0000000001d34bce in folly::detail::futexWait<std::atomic<unsigned int> > (futex=0x7f1c3ac2ef90, expected=137, waitMask=1) at buck-out/v2/gen/fbcode/110b607930331a92/folly/detail/__futex__/headers/folly/detail/Futex-inl.h:96
  #4  folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever::doWait (this=<optimized out>, futex=..., expected=137, waitMask=1) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:718
  #5  folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::futexWaitForZeroBits<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, state=@0x7ffd2d5be924: 118379408, goal=128, waitMask=1, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1184
  #6  0x0000000001cd42b2 in folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::yieldWaitForZeroBits<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, state=@0x7ffd2d5be924: 118379408, goal=128, waitMask=1, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1151
  #7  folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::waitForZeroBits<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, state=@0x7ffd2d5be924: 118379408, goal=128, waitMask=1, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1109
  #8  0x0000000001e7e14c in folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::lockSharedImpl<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, state=@0x7ffd2d5be924: 118379408, token=0x0, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1664
  #9  folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::lockSharedImpl<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::WaitForever> (this=0x7f1c3ac2ef90, token=0x0, ctx=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:1356
  #10 folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>::lock_shared (this=0x7f1c3ac2ef90) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__shared_mutex__/headers/folly/SharedMutex.h:495
  #11 std::shared_lock<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> >::shared_lock (this=<optimized out>, __m=...) at fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/shared_mutex:727
  #12 0x0000000002d765fd in folly::LockedPtr<folly::Synchronized<watchman::Publisher::state, folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> > const, folly::detail::SynchronizedLockPolicy<(folly::detail::SynchronizedMutexLevel)2, (folly::detail::SynchronizedMutexMethod)0> >::doLock<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault>, std::shared_lock<folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> >, folly::detail::SynchronizedLockPolicy<(folly::detail::SynchronizedMutexLevel)2, (folly::detail::SynchronizedMutexMethod)0>, 0> (mutex=...) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__synchronized__/headers/folly/Synchronized.h:1493
  #13 folly::LockedPtr<folly::Synchronized<watchman::Publisher::state, folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> > const, folly::detail::SynchronizedLockPolicy<(folly::detail::SynchronizedMutexLevel)2, (folly::detail::SynchronizedMutexMethod)0> >::LockedPtr (this=0x7ffd2d5be968, parent=<optimized out>) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__synchronized__/headers/folly/Synchronized.h:1272
  #14 folly::SynchronizedBase<folly::Synchronized<watchman::Publisher::state, folly::SharedMutexImpl<false, void, std::atomic, folly::SharedMutexPolicyDefault> >, (folly::detail::SynchronizedMutexLevel)2>::rlock (this=<optimized out>) at buck-out/v2/gen/fbcode/110b607930331a92/folly/__synchronized__/headers/folly/Synchronized.h:229
  #15 watchman::Publisher::hasSubscribers (this=<optimized out>) at fbcode/watchman/PubSub.cpp:117
  #16 0x0000000002ecac20 in watchman::Log::log<char const (&) [59]> (this=<optimized out>, level=level@entry=watchman::ABORT, args=...) at buck-out/v2/gen/fbcode/110b607930331a92/watchman/__logging__/headers/watchman/Logging.h:42
  #17 0x0000000002ec9b24 in watchman::log<char const (&) [59]> (level=watchman::ABORT, args=...) at buck-out/v2/gen/fbcode/110b607930331a92/watchman/__logging__/headers/watchman/Logging.h:121
  #18 (anonymous namespace)::terminationHandler () at fbcode/watchman/SignalHandler.cpp:165
  #19 0x00007f1c3b0c7b3a in __cxxabiv1::__terminate (handler=<optimized out>) at ../../.././libstdc++-v3/libsupc++/eh_terminate.cc:48
  #20 0x00007f1c3b0c7ba5 in std::terminate () at ../../.././libstdc++-v3/libsupc++/eh_terminate.cc:58
  #21 0x0000000002d8cde1 in std::thread::~thread (this=0x7f1c3ac2ef90) at fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_thread.h:152
  #22 0x00007f1c3b3cc8f8 in __run_exit_handlers (status=1, listp=0x7f1c3b598658 <__exit_funcs>, run_list_atexit=<optimized out>, run_dtors=<optimized out>) at exit.c:113
  #23 0x00007f1c3b3cca0a in __GI_exit (status=<optimized out>) at exit.c:143
  #24 0x00007f1c3b3b165e in __libc_start_call_main (main=0x2d11220 <main(int, char**)>, argc=2, argv=0x7ffd2d5bec78) at ../sysdeps/nptl/libc_start_call_main.h:74
  #25 0x00007f1c3b3b1718 in __libc_start_main_impl (main=0x2d11220 <main(int, char**)>, argc=2, argv=0x7ffd2d5bec78, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffd2d5bec68) at ../csu/libc-start.c:409
  #26 0x0000000002d0e181 in _start () at ../sysdeps/x86_64/start.S:116

Reviewed By: xavierd

Differential Revision: D40166374

fbshipit-source-id: 7017e20234e5e0a9532eb61a63ac49ac0020d443
facebook-github-bot pushed a commit that referenced this issue Oct 14, 2022
… or security patches (#2)

Summary:
X-link: hhvm/hhvm-staging#2

Pull Request resolved: #9259

[hhvm/hhvm-staging](https://github.com/hhvm/hhvm-staging) and [facebook/hhvm](https://github.com/facebook/hhvm) contains identical code except for unlanded diffs and security patches. It is unnecessary to run duplicated CI for both repositories.

This diff conditionally disables GitHub Actions for hhvm/hhvm-staging except for either pull requests, which should be exported from unlanded diffs, or `HHVM-*` branches/tags, which might be security patches.

Reviewed By: alexeyt

Differential Revision: D40373603

fbshipit-source-id: 4dcbb11c89bbe9939ae12379ea9b8edb64a2f595
facebook-github-bot pushed a commit that referenced this issue Mar 8, 2023
Summary:
AsyncUDPSocket test cases are crashing when running under llvm15. During
debugging it seems that the issue is the fact that the code tries to allocated 0
size array. Changing the code to prevent such allocation.

This is not very clean why to fix, but I am not sure if there is better one.
Please let me know if you have any suggestions.

Sample crash:
```
$ buck test //folly/io/async/test:async_udp_socket_test
...
stdout:
Note: Google Test filter = AsyncUDPSocketTest.TestDetachAttach
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from AsyncUDPSocketTest
[ RUN      ] AsyncUDPSocketTest.TestDetachAttach

stderr:
fbcode/folly/io/async/AsyncUDPSocket.cpp:699:10: runtime error: variable length array bound evaluates to non-positive value 0
    #0 0x7f4d8ed93704 in folly::AsyncUDPSocket::writev(folly::SocketAddress const&, iovec const*, unsigned long, int) fbcode/folly/io/async/AsyncUDPSocket.cpp:698
    #1 0x7f4d8ed9081f in folly::AsyncUDPSocket::writeGSO(folly::SocketAddress const&, std::unique_ptr<folly::IOBuf, std::default_delete<folly::IOBuf>> const&, int) fbcode/folly/io/async/AsyncUDPSocket.cpp:528
    #2 0x7f4d8ed900b2 in folly::AsyncUDPSocket::write(folly::SocketAddress const&, std::unique_ptr<folly::IOBuf, std::default_delete<folly::IOBuf>> const&) fbcode/folly/io/async/AsyncUDPSocket.cpp:660
    #3 0x350a05 in AsyncUDPSocketTest_TestDetachAttach_Test::TestBody() fbcode/folly/io/async/test/AsyncUDPSocketTest.cpp:914
    #4 0x7f4d90dd1ad5 in testing::Test::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2682:50
    #5 0x7f4d90dd1ad5 in testing::Test::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2672:6
    #6 0x7f4d90dd1c64 in testing::TestInfo::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2861:14
    #7 0x7f4d90dd1c64 in testing::TestInfo::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2833:6
    #8 0x7f4d90dd2321 in testing::TestSuite::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:3015:31
    #9 0x7f4d90dd2321 in testing::TestSuite::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2993:6
    #10 0x7f4d90dd2b1e in testing::internal::UnitTestImpl::RunAllTests() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:5855:47
    #11 0x7f4d90dd1d87 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:2665:29
    #12 0x7f4d90dd1d87 in testing::UnitTest::Run() /home/engshare/third-party2/googletest/1.11.0/src/googletest/googletest/src/gtest.cc:5438:55
    #13 0x7f4d90d5c990 in RUN_ALL_TESTS() fbcode/third-party-buck/platform010/build/googletest/include/gtest/gtest.h:2490
    #14 0x7f4d90d5c618 in main fbcode/common/gtest/LightMain.cpp:20
    #15 0x7f4d8ea2c656 in __libc_start_call_main /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #16 0x7f4d8ea2c717 in __libc_start_main@GLIBC_2.2.5 /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../csu/libc-start.c:409:3
    #17 0x33ea60 in _start /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/x86_64/start.S:116
...
```

Reviewed By: russoue, dmm-fb

Differential Revision: D43858875

fbshipit-source-id: 93749bab17027b6dfc0dbc01b6c183e501a5494c
facebook-github-bot pushed a commit that referenced this issue Apr 21, 2023
…t error definitions

Summary:
It'd be helpful to be able to call `.source()` on a Thrift client error, as that would make writing dynamic retry policies far easier and allow them to be used for multiple Thrift client methods. Normally to enable this, we just need to add the `#[source]` attribute (from thiserror) to the inner value of our error enums. But this results in bad error chaining and duplicated error messages when printing out the error chain (say with `anyhow`).

I think the best solution here is to take a page out of [anyhow](https://docs.rs/anyhow/latest/anyhow/)'s book, specifically the `{:#}` semantics for `Display`. This way, we can omit printing the cause when printing with `{}` (and thus avoiding duplicate error messages when printing with `anyhow`), but one can choose to retain the behavior from before with `{:#}`.

Aside from this, there's also a few cleanups we need to do with how Thrift client errors are laid out to support `.source()`, mainly:
1. Add `#[derive(thiserror::Error)]` to `ApplicationException`
2. This then requires us to remove the `impl From<ApplicationException> for Error` in `errors.rs` as it conflicts with `anyhow`'s existing blanket impl of `From<T: std::error::Error>`
3. Removing #2 causes issues in `unionimpl.mustache` which does rely on that impl. However, `unionimpl`'s usage just returns a `ProtocolError` that wraps an `ApplicationException` that has a `ProtocolError` as an error code... so just flatten that to a new `ProtocolError` variant
4. It appears that this `unionimpl` was the only time we used the `ProtocolError::ApplicationException` variant, so just remove that. Feels weird to wrap an `ApplicationException` inside of a `ProtocolError` anyways.

inspired by https://sabrinajewson.org/blog/errors#guidelines-for-good-errors.

Reviewed By: edward-shen

Differential Revision: D44808129

fbshipit-source-id: 051c4c499bc2aa309e6319e6da59ffb3788a55a3
facebook-github-bot pushed a commit that referenced this issue May 5, 2023
Summary:
Generated C++ Thrift code uses method names in a couple formats:
1. `<service name>.[<interaction name>.]<method name>` when configuring context
stacks, both on the [client](https://github.com/facebook/fbthrift/blob/5d86ee0072e56c64bfba8061e0f790fd3f17e362/thrift/compiler/generate/templates/cpp2/service_async_client_cpp/function_implementation.mustache#L85-L89) and the [server](https://github.com/facebook/fbthrift/blob/5d86ee0072e56c64bfba8061e0f790fd3f17e362/thrift/compiler/generate/templates/cpp2/service_tcc/process_and_return.mustache#L77).
2. [`[<interaction name>.]<method name>`](https://github.com/facebook/fbthrift/blob/5d86ee0072e56c64bfba8061e0f790fd3f17e362/thrift/compiler/generate/templates/cpp2/common/function_name.mustache#L17) for the [response envelope](https://github.com/facebook/fbthrift/blob/5d86ee0072e56c64bfba8061e0f790fd3f17e362/thrift/compiler/generate/templates/cpp2/service_tcc/process_and_return.mustache#L293) (if
applicable), the [read hook](https://github.com/facebook/fbthrift/blob/5d86ee0072e56c64bfba8061e0f790fd3f17e362/thrift/compiler/generate/templates/cpp2/service_tcc/process_and_return.mustache#L82), and the [write hook](https://github.com/facebook/fbthrift/blob/5d86ee0072e56c64bfba8061e0f790fd3f17e362/thrift/compiler/generate/templates/cpp2/service_tcc/process_and_return.mustache#L231).

Currently, Rust uses `<interaction name or service name>.<method name>` for both
of the above. The decision to use the interaction name or service name is based
on whether the method is part of an interaction - if it is, the interaction name
is used, otherwise the service name is used.

This diff updates Rust to be consistent with C++ in #1 and #2.

Reviewed By: jsgf

Differential Revision: D45322484

fbshipit-source-id: 38e2604921e69208cdcb983e8def71954b212b41
facebook-github-bot pushed a commit that referenced this issue Jul 28, 2023
Summary:
We have [at least] three redundant ADTs for representing Facts:
1. Rust hackc::FileFacts designed for interop with C++ FileFacts (#3)
2. Rust facts::Facts, carefully crafted to produce a specific JSON format
3. C++ HPHP::Facts::FileFacts constructed from folly::dynamic (from #2 JSON)

this diff massages #1 to get ready to replace #2 and #3 along with the
unnecessary representation changes that go with them.

Reviewed By: aorenste

Differential Revision: D46969023

fbshipit-source-id: 95f989774c995f094e216d6dc95f8a88aa0b2f06
facebook-github-bot pushed a commit that referenced this issue Jul 28, 2023
Summary:
For array access (e.g. `$x['a']`) through a value whose type is a type variable (e.g. `$x : #0`), we use lower bounds on the type variable (e.g. `Map<string,int> <: #0`) to "narrow" the possible solutions in order to type-check the access. Here, for example, we would use the lower bound to generate a type `KeyedContainer<#1,#2>` that, roughly speaking, is the widest type that supports the operation that is compatible with the lower bound constraint.

We generate a subtype assertion between the lower bound and the "widened" type i.e. `Map<string,int> <: KeyedContainer<#1, #2>`. Note the type variables. Here, `#2` has no constraints, and so solves to `mixed`. In the case that the original type variable has an *upper bound* that supports dynamic (e.g. `supportdyn<mixed>`), this produces code that fails our "pessimisation" conjecture (see new test). Instead, we can employ a heuristic: if the lower bound does support dynamic, then assert that the widened type also supports dynamic. We also generalize the treatment of open shapes, generating a type variable for the "unknown fields" (which typically will solve to `supportdyn<mixed>`) rather than fixing it to be `mixed`.

Differential Revision: D46872355

fbshipit-source-id: 5ad70f81aa92ab4278c6e68f6ea5b87c2f2b60fa
facebook-github-bot pushed a commit that referenced this issue Jan 29, 2024
Summary:
In {D53166614} and {D53166618}, we need to make either of 2 choices:

1. **Every** `thrift_library` gets a clients and services crate, regardless of whether the target contains any service definitions

2. The clients and services crates only exist if the target defines nonzero services

As far as I know, it's impossible to do #2 based on whether the target *actually* contains services in its Thrift source code, only based on whether the `thrift_library` macro call lists any service names.

https://www.internalfb.com/code/fbsource/[0fa600724c20b87b38d536611b742c8287322f75]/fbcode/zippydb/if/TARGETS?lines=9-16%2C32-34%2C46

So far for Rust we have not paid attention to the listed service names in the Buck target, but the other languages look at it for a similar purpose as this.

On the basis that only 1252 out of 6042 Thrift targets in fbcode today contain services, I think limiting the clients and services crates is the right call to keep the Buck graph small. We can revisit down the line if there is any reason this turns out to be annoying.

This diff eliminates the `client` and `server` modules of Thrift library targets that do not contain any services according to Buck. Previously these used to be either empty modules, or actually nonempty modules if there were services defined in the Thrift source code but not declared in the Buck target.

Based on check_all.sh, there are zero occurrences of Rust code importing a service from a Thrift library for which the Buck target does not declare any services.

Reviewed By: zertosh, shayne-fletcher

Differential Revision: D53189031

fbshipit-source-id: e4fa7ffee4ead76802bebbc92ad514ff1d24d3b2
facebook-github-bot pushed a commit that referenced this issue Feb 8, 2024
Summary: When logging subtype assertions, indicate whether the assertion was the result of transitive closure e.g. asserting `vec<int> <: #1` and then `#1 <: vec<#2>`.

Differential Revision: D53524300

fbshipit-source-id: f49121b8f7187a30bca3d34722c33c4d52be117e
facebook-github-bot pushed a commit that referenced this issue Apr 27, 2024
Summary:
ThreadLocalDetailTest.MultiThreadedTest had a data race on std::vector because multiple threads were accessing and modifying the std::vector without a mutex. I update the code to use indexes such that the vector is not updated concurrently.

Previous run failure that this fixes:

```
stderr:
==================
WARNING: ThreadSanitizer: data race (pid=1249005)
  Write of size 8 at 0x7fff195936d0 by main thread:
    #0 std::vector<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>, std::allocator<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>>>::pop_back() fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:1228 (thread_local_detail_test+0x116c28)
    #1 folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody() fbcode/folly/detail/test/ThreadLocalDetailTest.cpp:121 (thread_local_detail_test+0x101aba)
    #2 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) fbsource/src/gtest.cc:2675 (libthird-party_googletest_1.14.0_gtest.so+0xacf3c)
    #3 testing::Test::Run() fbsource/src/gtest.cc:2692 (libthird-party_googletest_1.14.0_gtest.so+0xacb9c)
    #4 testing::TestInfo::Run() fbsource/src/gtest.cc:2841 (libthird-party_googletest_1.14.0_gtest.so+0xafa4a)
    #5 testing::TestSuite::Run() fbsource/src/gtest.cc:3020 (libthird-party_googletest_1.14.0_gtest.so+0xb4303)
    #6 testing::internal::UnitTestImpl::RunAllTests() fbsource/src/gtest.cc:5925 (libthird-party_googletest_1.14.0_gtest.so+0xd105e)
    #7 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) fbsource/src/gtest.cc:2675 (libthird-party_googletest_1.14.0_gtest.so+0xd08f1)
    #8 testing::UnitTest::Run() fbsource/src/gtest.cc:5489 (libthird-party_googletest_1.14.0_gtest.so+0xd037e)
    #9 RUN_ALL_TESTS() fbsource/gtest/gtest.h:2317 (libcommon_gtest_light_main.so+0x22a7)
    #10 main fbcode/common/gtest/LightMain.cpp:20 (libcommon_gtest_light_main.so+0x2131)

  Previous read of size 8 at 0x7fff195936d0 by thread T123:
    #0 std::vector<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>, std::allocator<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>>>::size() const fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:919 (thread_local_detail_test+0x11b0bd)
    #1 std::vector<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>, std::allocator<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>>>::operator[](unsigned long) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:1045 (thread_local_detail_test+0x11d101)
    #2 folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0::operator()() const fbcode/folly/detail/test/ThreadLocalDetailTest.cpp:97 (thread_local_detail_test+0x11d08c)
    #3 void std::__invoke_impl<void, folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>(std::__invoke_other, folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0&&) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/invoke.h:61 (thread_local_detail_test+0x11cf95)
    #4 std::__invoke_result<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>::type std::__invoke<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>(folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0&&) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/invoke.h:96 (thread_local_detail_test+0x11cf05)
    #5 void std::thread::_Invoker<std::tuple<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>>::_M_invoke<0ul>(std::_Index_tuple<0ul>) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_thread.h:253 (thread_local_detail_test+0x11cebd)
    #6 std::thread::_Invoker<std::tuple<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>>::operator()() fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_thread.h:260 (thread_local_detail_test+0x11ce65)
    #7 std::thread::_State_impl<std::thread::_Invoker<std::tuple<folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody()::$_0>>>::_M_run() fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_thread.h:211 (thread_local_detail_test+0x11cd79)
    #8 execute_native_thread_routine /home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/src/c++11/../../../.././libstdc++-v3/src/c++11/thread.cc:82:18 (libstdc++.so.6+0xdf4e4) (BuildId: 452d1cdae868baeeb2fdf1ab140f1c219bf50c6e)

  Location is stack of main thread.

  Location is global '??' at 0x7fff19575000 ([stack]+0x1e6d0)

  Thread T123 (tid=1249233, running) created by main thread at:
    #0 pthread_create <null> (thread_local_detail_test+0x156bef)
    #1 __gthread_create /home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/include/x86_64-facebook-linux/bits/gthr-default.h:663:35 (libstdc++.so.6+0xdf80e) (BuildId: 452d1cdae868baeeb2fdf1ab140f1c219bf50c6e)
    #2 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State>>, void (*)()) /home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/src/c++11/../../../.././libstdc++-v3/src/c++11/thread.cc:147:37 (libstdc++.so.6+0xdf80e)
    #3 folly::threadlocal_detail::ThreadLocalDetailTest_MultiThreadedTest_Test::TestBody() fbcode/folly/detail/test/ThreadLocalDetailTest.cpp:93 (thread_local_detail_test+0x1015cd)
    #4 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) fbsource/src/gtest.cc:2675 (libthird-party_googletest_1.14.0_gtest.so+0xacf3c)
    #5 testing::Test::Run() fbsource/src/gtest.cc:2692 (libthird-party_googletest_1.14.0_gtest.so+0xacb9c)
    #6 testing::TestInfo::Run() fbsource/src/gtest.cc:2841 (libthird-party_googletest_1.14.0_gtest.so+0xafa4a)
    #7 testing::TestSuite::Run() fbsource/src/gtest.cc:3020 (libthird-party_googletest_1.14.0_gtest.so+0xb4303)
    #8 testing::internal::UnitTestImpl::RunAllTests() fbsource/src/gtest.cc:5925 (libthird-party_googletest_1.14.0_gtest.so+0xd105e)
    #9 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) fbsource/src/gtest.cc:2675 (libthird-party_googletest_1.14.0_gtest.so+0xd08f1)
    #10 testing::UnitTest::Run() fbsource/src/gtest.cc:5489 (libthird-party_googletest_1.14.0_gtest.so+0xd037e)
    #11 RUN_ALL_TESTS() fbsource/gtest/gtest.h:2317 (libcommon_gtest_light_main.so+0x22a7)
    #12 main fbcode/common/gtest/LightMain.cpp:20 (libcommon_gtest_light_main.so+0x2131)

ThreadSanitizer: data race fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/stl_vector.h:1228 in std::vector<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>, std::allocator<std::unique_ptr<folly::test::Barrier, std::default_delete<folly::test::Barrier>>>>::pop_back()
==================
ThreadSanitizer: reported 1 warnings
```

Reviewed By: yfeldblum

Differential Revision: D56643303

fbshipit-source-id: cc364817ae79bc6418cc07faa35e1bcd21830c66
facebook-github-bot pushed a commit that referenced this issue Jun 21, 2024
Summary:
# Context
When Enum Class Labels were implemented, they were restricted to being used in only function call position. This restriction was in place because it utilized a specialized form of inference that is implemented within `Typing.EnumClassLabelOps`. We want to lift this restriction and utilize more standard inference techniques for two reasons:

1. There is a desire to utilize enum class labels for other use cases besides passing immediately to a function. For instance, having a vec of labels that are iterated over and used to invoke a function or to use labels as the type of an XHP attribute.

2. The type constant in enum class feature is currently unsound. This was attempted to be fixed in D54439754, but that approach is incomplete. For a more complete solution we need more context than is available using the current `Typing.EnumClassLabelOps.expand` function

There is also a desire to explore usage of labels for other purposes besides mapping to enum class labels.

# Solution

This stack of diffs deletes the ad-hoc `Typing.EnumClassLabelOps` based inference for enum class labels. Instead it introduces a new locl type `Tlabel` and a new constraint type `Thas_const`.

## Definition
The new locl type is defined as `Tlabel of string` where `string` is the name of the label. I.e. for the expression `#Foo`, it would have the type `Tlabel "Foo"`.

The new constraint type is defined as `Thas_const of { name: string; ty: locl_ty }` where `name` is the name of the class constant and `ty` is the type ascribed to the class constant.

## Subtyping
The `Tlabel` type has a subtype relationship with the `HH\EnumClass\Label` type. Specifically

```
E <: HasConst(A, HH\MemberOf<E, T>)
---------------------------------- :: Label-Sub-EnumClassLabel
#A <: HH\EnumClass\Label<E, T>
```

`Label-Sub-EnumClassLabel` states a label `#A` is a subtype of an `EnumClass\Label<E, T>` if `E` has a class constant named `A` whose declared type is a subtype of `MemberOf<E, T>`.

The other subtyping rule added is:
```
LookUpConstType(C, Id) <: T
----------------------- :: Class-Sub-HasConst
C <: HasConst(Id, T)
```
`Class-Sub-HasConst` states assuming `C` is a class type, it is a subtype of `HasConst(Id, T)` if the declaration of `C` contains a class constant `Id` whose declared type is a subtype of `T`. Though not stated in this rule, `LookUpConstType` can reference the `this_ty` that is bound in the environment so we can properly instantiate the `this` type when localizing the decl type associated with `C::Id`. This is what allows us to address the incompleteness introduced by D54439754

## Typing
With the above typing rules we can simply give the expression `#A` the type `Tlabel "A"` and rely on the above subtyping rules to give us the behavior we desire. This allows us to give a type to an anonymous label expression at any position, meaning we can lift the restriction on them appearing only as function arguments

# Alternatives Considered
Adding a new locl type isn't a small change and I considered alternatives to avoid having to do this. I started with a single `Thas_label` constraint type that is similar to the current `Thas_const` type. In typing I would then generate a type `EnumClass\Label<#1, #2>` with the added constraint of `#1  <: Thas_label(Id, #2)`. While it worked mostly well, it ran into issues due to the second type parameter of `EnumClass\Label` being invariant. This meant we could never early solve that type variable and led to incompleteness.

I tried a few other changes attempts, but none had the desired affect so I settled on adding the `Tlabel` type as this diff does.

# This Diff
1. In this diff I add the `Tlabel` and `Thas_const` types
2. In D55829619 I implement the subtyping and typing changes described above

Reviewed By: viratyosin

Differential Revision: D55829620

fbshipit-source-id: 43c7c30280d83f669025fe0d34248c67d54c94d0
facebook-github-bot pushed a commit that referenced this issue Jun 25, 2024
Summary:
This implements the typing and subtyping changes described in D55829620.

There were a few subtle changes needed to ensure all test pass.

1. For autocomplete, when `#AUTO332` is encountered in a function argument position we didn't use to produce a `Hole` in the TAST. Now we do, so I had to update some code in autocompleteService.ml to strip out the Hole.

2. Zoncolan has code that expects for the pattern `f(#Label)` that `#Label` would have the type `EnumClass\Label`. This diff changes it to be a `Tlabel` type. To better support these cases, I'll try to infer a `EnumClass\Label` type when I can. This correlates to cases when an expected type is providing when typing an `EnumClassLabel` expression. In that case we will use the expected type instead of creating a `Tlabel` type for the expression

3. Since I am deleting `Typing.EnumClassLabelOps` I also needed to update the typing for `E#A` expressions. This is done by checking `E <: HasConst(A, MemberOf<#1, #2>)`, solving the type variables, and creating the type `EnumClass\Label<#1, #2>` as the type.

Reviewed By: viratyosin

Differential Revision: D55829619

fbshipit-source-id: 2e0abaf62b5fd48e8511cbb505429ecc4e2006cb
facebook-github-bot pushed a commit that referenced this issue Jul 10, 2024
Summary:
I ran across this testing something. `Vec\intersect()` doesn't work when the first parameter is a generator:

```
fbdbg> $s = Streams::filter(Vec\range(1, 10), $x ==> true)
fbdbg> =Vec\intersect($s, vec[5])
Hit a php exception : exception 'Exception' with message 'Generator is already finished' in /data/sandcastle/boxes/www/flib/core/hack/lib/hsl_generated/vec/select.php:168
Stack trace:
#0 (): Generator->rewind()
#1 (): HH\Lib\Vec\filter()
#2 /data/sandcastle/boxes/www/flib/core/hack/lib/hsl_generated/vec/select.php(168): HH\Lib\Vec\intersect()
#3 (1): FlibSL\Vec\intersect()
#4 (1): include()
#5 (): include()
#6 {main}
Hit a php exception : exception 'Exception' with message 'Generator is already finished' in /data/sandcastle/boxes/www/flib/core/hack/lib/hsl_generated/vec/select.php:168
Stack trace:
#0 (): Generator->rewind()
#1 (): HH\Lib\Vec\filter()
#2 /data/sandcastle/boxes/www/flib/core/hack/lib/hsl_generated/vec/select.php(168): HH\Lib\Vec\intersect()
#3 (1): FlibSL\Vec\intersect()
#4 (1): include()
#5 (): include()
#6 {main}
Failed to evaluate expression
```

The cause is that it iterates over the generator again after `Keyset\intersect()` has been called.

https://www.internalfb.com/code/fbsource/[4e26eed969060508bfbd3c4f675e018ca367c556]/fbcode/hphp/hsl/src/vec/select.php?lines=186-189

Reviewed By: viratyosin

Differential Revision: D59356423

fbshipit-source-id: 8ec1b5d00183d09606728c97d61bf30a26bfdf1c
This issue was closed.
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

No branches or pull requests

4 participants