condition statement is not correct in sockaddrToInetAddress
Closed this issue · 36 comments
clang will simplify the condition branch to a single compare instrustions for "if (ss.ss_family == AF_INET6 && IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr)) " in libcore/luni/src/main/native/NetworkUtilities.cpp.
if (ss.ss_family == AF_INET6 && IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr))
will be complied to:
> 0000000000000012 <.Lpcrel_hi0>:
> 12: 97 06 00 00 auipc a3, 0
> 16: 83 ba 06 00 ld s5, 0(a3)
> 1a: 83 b6 0a 00 ld a3, 0(s5)
> 1e: ae 84 mv s1, a1
> 20: 36 e5 sd a3, 136(sp)
> 22: 8c 45 lw a1, 8(a1)
> 24: d8 44 lw a4, 12(s1)
> 26: 83 d6 04 00 lhu a3, 0(s1)
> 2a: d9 8d or a1, a1, a4
> 2c: 98 48 lw a4, 16(s1)
> 2e: 93 c7 a6 00 xori a5, a3, 10
> 32: dd 8d or a1, a1, a5
> 34: c1 77 lui a5, 1048560
> 36: 3d 8f xor a4, a4, a5
> 38: d9 8d or a1, a1, a4
> 3a: 32 89 mv s2, a2
> 3c: 2a 84 mv s0, a0
> 3e: 81 e1 bnez a1, 0x3e <.Lpcrel_hi0+0x2c>In these case, the clang simplify condition branch with a bnez(20->3e). But sockaddr_storage& ss may be a address of IPv4, the IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr) will be access a address which is overlow(2c).
I think if we want to use "&&" for truncation, then we should change the IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr) condition to a function call. Alternatively, we can write this condition inside the if statement.
@enh-google
this is a clang bug, no? @appujee
if (ss.ss_family == AF_INET6 && IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr)) is essentially:
if (ss.ss_family == 10 &&
((((&sin6.sin6_addr)->in6_u.u6_addr32[0]) == 0) &&
(((&sin6.sin6_addr)->in6_u.u6_addr32[1]) == 0) &&
(((&sin6.sin6_addr)->in6_u.u6_addr32[2]) == __builtin_bswap32(0x0000ffff)))) {
I think if we want to use "&&" for truncation
Can you point to where '&&' is being used for truncation. ISTM it is only being used for boolean operations.
if "ss.ss_family == 10"(A) false, the "IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr)"(B) should not be executed. "&&" means the condtions should be executed from left to right.
from the disassembly, it acctually execute A and get result in a1, then execute B and get result in a4. It does "or a1, a1, a4" + "bnez a1, 0x3e" to simplify the condition branch.
I think it is expected in compiler, because condtion B is reading data from memory, it doesn't destroy anything. If B is a CALL or WRITE, it won't be optimized.
The reason why this optimization is not suitable here is that"((&sin6.sin6_addr)->in6_u.u6_addr32[2])" may be out of bounds (2c: 98 48 lw a4, 16(s1)), because if the parameter "ss" is allocated according for the address of IPv4, it could cause the out-of-bounds access. here is the struct of ipv4 and ipv6:
this is a clang bug, no? @appujee
I think it is not a clang bug. it is a common optimization in the middle IR. Both aarch64 and RISCV will do this optizimation:
It looks strange to me.
What happens if you change the code to
if (ss.ss_family == AF_INET6) {
const sockaddr_in6& sin6 = reinterpret_cast<const sockaddr_in6&>(ss);
if (IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr)) {
// Copy the IPv6 address into the temporary sockaddr_storage.
sockaddr_storage tmp;
memset(&tmp, 0, sizeof(tmp));
memcpy(&tmp, &ss, sizeof(sockaddr_in6));
// Unmap it into an IPv4 address.
sockaddr_in& sin = reinterpret_cast<sockaddr_in&>(tmp);
sin.sin_family = AF_INET;
sin.sin_port = sin6.sin6_port;
memcpy(&sin.sin_addr.s_addr, &sin6.sin6_addr.s6_addr[12], 4);
// Do the regular conversion using the unmapped address.
return sockaddrToInetAddress(env, tmp, port);
}
}
So the reinterpret_cast only happens if ss.ss_family == AF_INET6
So the reinterpret_cast only happens if ss.ss_family == AF_INET6
but that's literally what && means, no? i feel like i'm missing something here...
"just because clang miscompiles for arm64 too doesn't mean this isn't a clang bug". i don't see how this is any different from p != null && *p == 3 incorrectly unconditionally dereferencing p. or i < MAX && a[i] == 3, given that the ss_family check is basically a size check.
The current code does
const sockaddr_in6& sin6 = reinterpret_cast<const sockaddr_in6&>(ss);
if (ss.ss_family == AF_INET6 && IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr))
No fields are accessed until after the && but the cast did occur before. Is it valid to do a reinterpret_cast without knowing for sure that the cast is valid?
No fields are accessed until after the && but the cast did occur before. Is it valid to do a reinterpret_cast without knowing for sure that the cast is valid?
ah, sorry, i hadn't clicked through to the full source ... i was just looking at the one-line snippet /facepalm
but where exactly is this causing trouble? afaict from manual inspection of the callers, there's no caller that isn't passing in sockaddr_storage here (which is the struct that's guaranteed to be at least as large as any specific sockaddr).
so i'm not sure what the specific claim is here about what is wrong? given what topperc just said, clang's optimization is actually fine (assuming clang has reason to believe that "speculating" those loads is a win), and the function itself is fine (unless someone writes a caller that passes in something smaller than a sockaddr_storage), and the callers themselves are fine (because afaict they're not doing that), and even if they did, that would be the real bug --- "i lied to this function and passed it something that wasn't a sockaddr_storage, and things blew up, but obviously that's my fault".
(the closest i can come to a bug here is "should we mark this case as unlikely?", but i actually don't have statistics on mapped addresses versus non-mapped to back that claim up :-) )
ok so here is a smaller repro case:https://godbolt.org/z/7b8dbMhvo (note i have renamed memcpy etc to prevent compiler from inlining). Looks like in both the cases (even when reinterpret cast is moved inside) the values are getting loaded speculatively.
ok so here is a smaller repro case:https://godbolt.org/z/7b8dbMhvo (note i have renamed memcpy etc to prevent compiler from inlining). Looks like in both the cases (even when reinterpret cast is moved inside) the values are getting loaded speculatively.
whereas if you build for arm64, it generates the code you'd expect to see (with or without [[unlikely]]):
sockaddrToInetAddress(void*, sockaddr_storage const&, int*): // @sockaddrToInetAddress(void*, sockaddr_storage const&, int*)
ldrh w8, [x1]
cmp w8, #10
b.ne .LBB0_4
...
ok so here is a smaller repro case:https://godbolt.org/z/7b8dbMhvo (note i have renamed memcpy etc to prevent compiler from inlining). Looks like in both the cases (even when reinterpret cast is moved inside) the values are getting loaded speculatively.
whereas if you build for arm64, it generates the code you'd expect to see (with or without
[[unlikely]]):sockaddrToInetAddress(void*, sockaddr_storage const&, int*): // @sockaddrToInetAddress(void*, sockaddr_storage const&, int*) ldrh w8, [x1] cmp w8, #10 b.ne .LBB0_4 ...
No fields are accessed until after the && but the cast did occur before. Is it valid to do a reinterpret_cast without knowing for sure that the cast is valid?
ah, sorry, i hadn't clicked through to the full source ... i was just looking at the one-line snippet /facepalm
but where exactly is this causing trouble? afaict from manual inspection of the callers, there's no caller that isn't passing in sockaddr_storage here (which is the struct that's guaranteed to be at least as large as any specific sockaddr).
so i'm not sure what the specific claim is here about what is wrong? given what topperc just said, clang's optimization is actually fine (assuming clang has reason to believe that "speculating" those loads is a win), and the function itself is fine (unless someone writes a caller that passes in something smaller than a sockaddr_storage), and the callers themselves are fine (because afaict they're not doing that), and even if they did, that would be the real bug --- "i lied to this function and passed it something that wasn't a sockaddr_storage, and things blew up, but obviously that's my fault".
(the closest i can come to a bug here is "should we mark this case as unlikely?", but i actually don't have statistics on mapped addresses versus non-mapped to back that claim up :-) )
The address was alloced in get_ai. The "afd->a_socklen" would be less then sizeof(struct sockaddr_storage).
here is an other allocation for ai, it would be fine.
hmmm... i was looking at the good site (which receives data from netd). i thought the bad site was dead code, but let me ask one of the networking people...
Oh there was a bug in my previous repro (non-void function does not return a value in all control paths). The modified one doesn't have bug when we move the reinterpret_cast inside the conditional.
Command line to repro the test:
$ ./bin/clang++ -std=c++20 -target riscv64-unknown-linux-gnu -O3 -march=rv64gcv a.cpp -o a.s -S -emit-llvm
Code: a.cpp
#include<string.h>
#include<stdint.h>
struct in6_u_t {
unsigned u6_addr32[4];
};
struct sin6_addr_t {
in6_u_t in6_u;
};
typedef unsigned short __u16;
typedef unsigned char __u8;
typedef __u16 __be16;
struct in6_addr {
union {
__u8 u6_addr8[16];
__be16 u6_addr16[8];
unsigned u6_addr32[4];
} in6_u;
};
struct sockaddr_in6 {
unsigned short int sin6_family;
__be16 sin6_port;
unsigned sin6_flowinfo;
struct in6_addr sin6_addr;
unsigned sin6_scope_id;
};
typedef unsigned short sa_family_t;
struct sockaddr_storage {
union {
struct {
sa_family_t ss_family;
char __data[128 - sizeof(sa_family_t)];
};
void* __align;
};
};
typedef uint32_t in_addr_t;
struct in_addr {
in_addr_t s_addr;
};
struct sockaddr_in {
unsigned sin_family;
unsigned sin_port;
struct in_addr sin_addr;
unsigned char __pad[16 - sizeof(short int) - sizeof(unsigned short int) - sizeof(struct in_addr)];
};
void foo(int);
void memset1(void* dest, int, int);
void memcpy1(void* dest, const void* src, int);
void* sockaddrToInetAddress(void* env, const sockaddr_storage& ss, int* port) {
const sockaddr_in6& sin6 = reinterpret_cast<const sockaddr_in6&>(ss);
if (ss.ss_family == 10 &&
((((&sin6.sin6_addr)->in6_u.u6_addr32[0]) == 0) &&
(((&sin6.sin6_addr)->in6_u.u6_addr32[1]) == 0) &&
(((&sin6.sin6_addr)->in6_u.u6_addr32[2]) == __builtin_bswap32(0x0000ffff)))) {
//if (ss.ss_family == AF_INET6 && IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr))
sockaddr_storage tmp;
memset1(&tmp, 0, sizeof(tmp));
memcpy1(&tmp, &ss, sizeof(sockaddr_in6));
sockaddr_in& sin = reinterpret_cast<sockaddr_in&>(tmp);
sin.sin_family = 2;
sin.sin_port = sin6.sin6_port;
memcpy1(&sin.sin_addr.s_addr, &sin6.sin6_addr.in6_u.u6_addr8[12], 4);
return sockaddrToInetAddress(env, tmp, port);
}
return env;
}
void* sockaddrToInetAddress_1(void* env, const sockaddr_storage& ss, int* port) {
if (ss.ss_family == 10) {
const sockaddr_in6& sin6 = reinterpret_cast<const sockaddr_in6&>(ss);
if ((((&sin6.sin6_addr)->in6_u.u6_addr32[0]) == 0) &&
(((&sin6.sin6_addr)->in6_u.u6_addr32[1]) == 0) &&
(((&sin6.sin6_addr)->in6_u.u6_addr32[2]) == __builtin_bswap32(0x0000ffff))) {
//if (ss.ss_family == AF_INET6 && IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr))
sockaddr_storage tmp;
memset1(&tmp, 0, sizeof(tmp));
memcpy1(&tmp, &ss, sizeof(sockaddr_in6));
sockaddr_in& sin = reinterpret_cast<sockaddr_in&>(tmp);
sin.sin_family = 2;
sin.sin_port = sin6.sin6_port;
memcpy1(&sin.sin_addr.s_addr, &sin6.sin6_addr.in6_u.u6_addr8[12], 4);
return sockaddrToInetAddress_1(env, tmp, port);
}
}
return env;
}This will fix RISC-V to stop trying so hard to use a single branch. Pretty sure I have this removed for our cores in our downstream branch.
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 241bc96766f2..6c4f84804d8e 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -1364,7 +1364,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
setPrefLoopAlignment(Subtarget.getPrefLoopAlignment());
// Jumps are expensive, compared to logic
- setJumpIsExpensive();
+ //setJumpIsExpensive();
setTargetDAGCombine({ISD::INTRINSIC_VOID, ISD::INTRINSIC_W_CHAIN,
ISD::INTRINSIC_WO_CHAIN, ISD::ADD, ISD::SUB, ISD::AND,
This is a bug in the code and not compiler. the reinterpret_cast is to a reference and in C++ references can't be null.
This is a bug in the code and not compiler. the reinterpret_cast is to a reference and in C++ references can't be null.
Is it a code bug? If the storage object is large enough to cover the ipv6 struct, then everything is fine isn't it?
This is a bug in the code and not compiler. the reinterpret_cast is to a reference and in C++ references can't be null.
Is it a code bug? If the storage object is large enough to cover the ipv6 struct, then everything is fine isn't it?
aiui the claim is that this code references off the end of a sockaddr_in. which can't happen on Android because bionic will only ever give you a sockaddr_storage, which is large enough that -- though returning invalid data[1] -- the "speculative" loads don't cause accesses off the end of the object.
my guess is that someone found this by testing on the host (with asan?). because even bionic-on-host (which doesn't use netd) will only allocate allocate "just enough" bytes rather than always allocating a full sockaddr_storage.
- which should be all zero even on a jemalloc device, because bionic uses calloc().
IMO once we take a reference to an object, all the loads from that reference should be valid (and hence can be speculated)
((((&sin6.sin6_addr)->in6_u.u6_addr32[0]) == 0) &&
(((&sin6.sin6_addr)->in6_u.u6_addr32[1]) == 0) &&
(((&sin6.sin6_addr)->in6_u.u6_addr32[2])
As per the OP
The reason why this optimization is not suitable here is that"((&sin6.sin6_addr)->in6_u.u6_addr32[2])" may be out of bounds (2c: 98 48 lw a4, 16(s1)), because if the parameter "ss" is allocated according for the address of IPv4, it could cause the out-of-bounds access. here is the struct of ipv4 and ipv6:
Once we have taken the reference, it is the programmer's responsibility to make sure the (contents of the) loads are valid. From a compiler's perspective there is enough storage (to a reference) that is legal to load from.
This will fix RISC-V to stop trying so hard to use a single branch. Pretty sure I have this removed for our cores in our downstream branch.
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp index 241bc96766f2..6c4f84804d8e 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -1364,7 +1364,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM, setPrefLoopAlignment(Subtarget.getPrefLoopAlignment()); // Jumps are expensive, compared to logic - setJumpIsExpensive(); + //setJumpIsExpensive(); setTargetDAGCombine({ISD::INTRINSIC_VOID, ISD::INTRINSIC_W_CHAIN, ISD::INTRINSIC_WO_CHAIN, ISD::ADD, ISD::SUB, ISD::AND,
i don't suppose there's a command-line flag for that? it seems like only the microcontroller folks actually want that? (and i'm worried how much other "works on arm64/x86-64" code is out there!)
This will fix RISC-V to stop trying so hard to use a single branch. Pretty sure I have this removed for our cores in our downstream branch.
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp index 241bc96766f2..6c4f84804d8e 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -1364,7 +1364,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM, setPrefLoopAlignment(Subtarget.getPrefLoopAlignment()); // Jumps are expensive, compared to logic - setJumpIsExpensive(); + //setJumpIsExpensive(); setTargetDAGCombine({ISD::INTRINSIC_VOID, ISD::INTRINSIC_W_CHAIN, ISD::INTRINSIC_WO_CHAIN, ISD::ADD, ISD::SUB, ISD::AND,i don't suppose there's a command-line flag for that? it seems like only the microcontroller folks actually want that? (and i'm worried how much other "works on arm64/x86-64" code is out there!)
There is! -mllvm -jump-is-expensive=false
This will fix RISC-V to stop trying so hard to use a single branch. Pretty sure I have this removed for our cores in our downstream branch.
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp index 241bc96766f2..6c4f84804d8e 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -1364,7 +1364,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM, setPrefLoopAlignment(Subtarget.getPrefLoopAlignment()); // Jumps are expensive, compared to logic - setJumpIsExpensive(); + //setJumpIsExpensive(); setTargetDAGCombine({ISD::INTRINSIC_VOID, ISD::INTRINSIC_W_CHAIN, ISD::INTRINSIC_WO_CHAIN, ISD::ADD, ISD::SUB, ISD::AND,i don't suppose there's a command-line flag for that? it seems like only the microcontroller folks actually want that? (and i'm worried how much other "works on arm64/x86-64" code is out there!)
There is!
-mllvm -jump-is-expensive=false
nice! i always wear belt and braces, so i'll do that too :-) (but we should probably look at getting upstream's default changed?)
but we should probably look at getting upstream's default changed?)
makes sense, at least for android-riscv and maybe for linux-riscv as well
aiui the claim is that this code references off the end of a sockaddr_in. which can't happen on Android because bionic will only ever give you a sockaddr_storage, which is large enough that -- though returning invalid data[1] -- the "speculative" loads don't cause accesses off the end of the object.
my guess is that someone found this by testing on the host (with asan?). because even bionic-on-host (which doesn't use netd) will only allocate allocate "just enough" bytes rather than always allocating a full sockaddr_storage.
The function is declared as taking a sockaddr_storage &. Are you saying there's some code that passes a smaller type to it?
after i claimed this couldn't happen on Android, the reporter replied:
The address was alloced in get_ai. The "afd->a_socklen" would be less then sizeof(struct sockaddr_storage).
here is an other allocation for ai, it would be fine.
the "here" link is the usual code in Android --- that's always a calloc()ed sockaddr_storage, and that's the code i was thinking of.
the "get_ai" link is what you get in the unlikely event you're running bionic on the host (so there's no netd to talk to), and that just uses the smallest struct it needs for the result it got from its direct DNS request. (i'm guessing they're actually using glibc rather than host bionic, although i think ART does also have some host bionic tests, so maybe not?)
okay, i've uploaded a couple of other fixes (basically the "move the cast" fix, plus a "take a sockaddr* instead" fix) and mailed the libcore folks with some background and suggest they pick whatever they're most comfortable with (since i'm unlikely to ever have the time to pick apart the circular dependency introduced by the "remove the duplication" fix).
meanwhile, https://android-review.googlesource.com/c/platform/build/soong/+/2853594 adds topperc's "don't do that" flag to the build.
after i claimed this couldn't happen on Android, the reporter replied:
The address was alloced in get_ai. The "afd->a_socklen" would be less then sizeof(struct sockaddr_storage).
here is an other allocation for ai, it would be fine.the "here" link is the usual code in Android --- that's always a calloc()ed sockaddr_storage, and that's the code i was thinking of.
the "get_ai" link is what you get in the unlikely event you're running bionic on the host (so there's no netd to talk to), and that just uses the smallest struct it needs for the result it got from its direct DNS request. (i'm guessing they're actually using glibc rather than host bionic, although i think ART does also have some host bionic tests, so maybe not?)
Actually, we got this issue on the devic, not on the host. I'll ask for more detail about the situation.
Actually, we got this issue on the devic, not on the host. I'll ask for more detail about the situation.
yes please.
the build change is in, and i'll be merging the libcore cleanups today, but if there is something i'm missing here, that's a bit worrying...
my guess is that you're seeing crashes in netd itself, which sets the environment variable to say "don't call out to netd" (obviously!)? i thought netd had switched to its own copy of this stuff by now (as part of becoming a mainline module), but i haven't heard back from the netd folks...
I added some logs in android_getaddrinfofornetcontext and get_ai, and add a backtrace in the sockaddrToInetAddress:
+#define TLOGE(...) syslog(LOG_ERR|LOG_AUTH,__VA_ARGS__);
+
typedef union sockaddr_union {
struct sockaddr generic;
struct sockaddr_in in;
@@ -587,6 +590,13 @@ android_getaddrinfofornetcontext(const char *hostname, const char *servname,
const struct addrinfo *hints, const struct android_net_context *netcontext,
struct addrinfo **res)
{
+ TLOGE("T-test android_getaddrinfofornetcontext hostname:%s",hostname);
+ TLOGE("T-test android_getaddrinfofornetcontext servname:%s",servname);
+ TLOGE("T-test android_getaddrinfofornetcontext hints size:%lu",sizeof(*hints));
+ TLOGE("T-test android_getaddrinfofornetcontext hints ai_flags:%d",hints->ai_flags);
+ TLOGE("T-test android_getaddrinfofornetcontext hints ai_family:%d",hints->ai_family);
+ TLOGE("T-test android_getaddrinfofornetcontext hints ai_socktype:%d",hints->ai_socktype);
+ TLOGE("T-test android_getaddrinfofornetcontext hints ai_protocol:%d",hints->ai_protocol);
struct addrinfo sentinel;
struct addrinfo *cur;
int error = 0;
@@ -862,6 +872,10 @@ static int
explore_null(const struct addrinfo *pai, const char *servname,
struct addrinfo **res)
{
+ TLOGE("T-test explore_null pai ai_flags:%d",pai->ai_flags);
+ TLOGE("T-test explore_null pai ai_family:%d",pai->ai_family);
+ TLOGE("T-test explore_null pai ai_socktype:%d",pai->ai_socktype);
+ TLOGE("T-test explore_null pai ai_protocol:%d",pai->ai_protocol);
int s;
const struct afd *afd;
struct addrinfo *cur;
@@ -1098,6 +1112,11 @@ get_canonname(const struct addrinfo *pai, struct addrinfo *ai, const char *str)
static struct addrinfo *
get_ai(const struct addrinfo *pai, const struct afd *afd, const char *addr)
{
+ TLOGE("T-test get_ai pai ai_flags:%d",pai->ai_flags);
+ TLOGE("T-test get_ai pai ai_family:%d",pai->ai_family);
+ TLOGE("T-test get_ai pai ai_socktype:%d",pai->ai_socktype);
+ TLOGE("T-test get_ai pai ai_protocol:%d",pai->ai_protocol);
+ TLOGE("T-test get_ai pai socklen:%d",afd->a_socklen);
char *p;
struct addrinfo *ai;
#include <nativehelper/ScopedLocalRef.h>
@@ -32,11 +38,39 @@
#include "JniConstants.h"
#include <log/log.h>
+#include <android/log.h>
+#define T_LOG_TAG "T-Test"
+#define TLOGE(...) __android_log_print(ANDROID_LOG_ERROR, T_LOG_TAG, __VA_ARGS__)
+
+void print_stack_trace() {
+ std::unique_ptr<Backtrace> backtrace(Backtrace::Create(BACKTRACE_CURRENT_PROCESS, BACKTRACE_CURRENT_THREAD, nullptr));
+ if (!backtrace->Unwind(0, nullptr)) {
+ TLOGE("Unwind failed");
+ return;
+ }
+
+ for (size_t i = 0; i < (backtrace->NumFrames() > 10 ? 10:backtrace->NumFrames()); i++) {
+ const backtrace_frame_data_t* frame = backtrace->GetFrame(i);
+ if (frame) {
+ TLOGE("#%02zu pc %016lx %s (%s+%lx) [%lx]",
+ i,
+ frame->pc,
+ frame->func_name.c_str(),
+ frame->map.name.c_str(),
+ frame->rel_pc,
+ frame->map.start);
+ }
+ }
+}
jobject sockaddrToInetAddress(JNIEnv* env, const sockaddr_storage& ss, jint* port) {
// Convert IPv4-mapped IPv6 addresses to IPv4 addresses.
// The RI states "Java will never return an IPv4-mapped address".
+ TLOGE("sockaddrToInetAddress ss ss_family:%d",ss.ss_family);
+ if (ss.ss_family ==AF_INET) {
+ print_stack_trace();
+ }
const sockaddr_in6& sin6 = reinterpret_cast<const sockaddr_in6&>(ss);
if (ss.ss_family == AF_INET6 && IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr)) {
// Copy the IPv6 address into the temporary sockaddr_storage.
And we got some logs here:
12-06 02:44:51.982 533 757 E system_server: T-test android_getaddrinfofornetcontext hostname:0.0.0.0
12-06 02:44:51.982 533 757 E system_server: T-test android_getaddrinfofornetcontext servname:(null)
12-06 02:44:51.983 533 757 E system_server: T-test android_getaddrinfofornetcontext hints size:48
12-06 02:44:51.983 533 757 E system_server: T-test android_getaddrinfofornetcontext hints ai_flags:4
12-06 02:44:51.983 533 757 E system_server: T-test android_getaddrinfofornetcontext hints ai_family:0
12-06 02:44:51.983 533 757 E system_server: T-test android_getaddrinfofornetcontext hints ai_socktype:0
12-06 02:44:51.984 533 757 E system_server: T-test android_getaddrinfofornetcontext hints ai_protocol:0
12-06 02:44:51.984 533 757 E system_server: T-test get_ai pai ai_flags:4
12-06 02:44:51.984 533 757 E system_server: T-test get_ai pai ai_family:2
12-06 02:44:51.984 533 757 E system_server: T-test get_ai pai ai_socktype:2
12-06 02:44:51.984 533 757 E system_server: T-test get_ai pai ai_protocol:17
12-06 02:44:51.984 533 757 E system_server: T-test get_ai pai socklen:16
12-06 02:44:51.985 533 757 E system_server: T-test get_ai pai ai_flags:4
12-06 02:44:51.985 533 757 E system_server: T-test get_ai pai ai_family:2
12-06 02:44:51.985 533 757 E system_server: T-test get_ai pai ai_socktype:1
12-06 02:44:51.985 533 757 E system_server: T-test get_ai pai ai_protocol:6
12-06 02:44:51.985 533 757 E system_server: T-test get_ai pai socklen:16
12-06 02:44:53.813 533 757 E T-Test : #00 pc 000000370ea97528 print_stack_trace() (/apex/com.android.art/lib64/libjavacore.so+13528) [370ea95000]
12-06 02:44:53.814 533 757 E T-Test : #1 pc 000000370ea97670 sockaddrToInetAddress(_JNIEnv*, sockaddr_storage const&, int*) (/apex/com.android.art/lib64/libjavacore.so+13670) [370ea95000]
12-06 02:44:53.814 533 757 E T-Test : #2 pc 000000370eaa379a Linux_android_getaddrinfo(_JNIEnv*, _jobject*, _jstring*, _jobject*, int) (/apex/com.android.art/lib64/libjavacore.so+1f79a) [370ea95000]
12-06 02:44:53.814 533 757 E T-Test : #3 pc 00000000716d1174 art_jni_trampoline (/apex/com.android.art/javalib/riscv64/boot-core-libart.oat+15174) [716ca000]
12-06 02:44:53.814 533 757 E T-Test : #4 pc 000000007171dc10 libcore.io.BlockGuardOs.android_getaddrinfo (/apex/com.android.art/javalib/riscv64/boot-core-libart.oat+61c10) [716ca000]
12-06 02:44:53.814 533 757 E T-Test : #5 pc 00000000716edf90 libcore.net.InetAddressUtils.parseNumericAddressNoThrow (/apex/com.android.art/javalib/riscv64/boot-core-libart.oat+31f90) [716ca000]
12-06 02:44:53.814 533 757 E T-Test : #6 pc 00000000716eddf0 libcore.net.InetAddressUtils.parseNumericAddress (/apex/com.android.art/javalib/riscv64/boot-core-libart.oat+31df0) [716ca000]
12-06 02:44:53.814 533 757 E T-Test : #7 pc 000000371a8bf896 nterp_helper (/apex/com.android.art/lib64/libart.so+8bf896) [371a372000]
12-06 02:44:53.814 533 757 E T-Test : #8 pc 000000372ee26f30 android.net.InetAddresses.parseNumericAddress (/apex/com.android.tethering/javalib/framework-connectivity.jar+22f30) [372ee04000]
12-06 02:44:53.814 533 757 E T-Test : #9 pc 000000371a8bf81e nterp_helper (/apex/com.android.art/lib64/libart.so+8bf81e) [371a372000]
It looks like system_server will request for address info for ipv4 and call sockaddrToInetAddress. But I'm still not clear why the system_server do this. Do you havae any suggestion?
It looks like system_server will request for address info for ipv4 and call sockaddrToInetAddress. But I'm still not clear why the system_server do this. Do you havae any suggestion?
thanks for digging into this! from the "framework-connectivity.jar" in there, i'm assuming that some of the dns resolver code runs in system_server? i'll forward this to the connectivity folks and see what they say...
Jump marked not expensive by default in trunk: llvm/llvm-project@2c18570 Thanks to Craig for putting this patch.
The current version clang-r510928(in aosp) doesn't. Upcoming clang should have it.
Merged.