Skip to content

Commit 87c0dbb

Browse files
committed
Merging r357376 and r359120:
------------------------------------------------------------------------ r357376 | labath | 2019-04-01 01:11:46 -0700 (Mon, 01 Apr 2019) | 27 lines [Linux/x86] Fix writing of non-gpr registers on newer processors Summary: We're using ptrace(PTRACE_SETREGSET, NT_X86_XSTATE) to write all non-gpt registers on x86 linux. Unfortunately, this method has a quirk, where the kernel rejects all attempts to write to this area if one supplies a buffer which is smaller than the area size (even though the kernel will happily accept partial reads from it). This means that if the CPU supports some new registers/extensions that we don't know about (in my case it was the PKRU extension), we will fail to write *any* non-gpr registers, even those that we know about. Since this is a situation that's likely to appear again and again, I add code to NativeRegisterContextLinux_x86_64 to detect the runtime size of the area, and allocate an appropriate buffer. This does not mean that we will start automatically supporting all new extensions, but it does mean that the new extensions will not prevent the old ones from working. This fixes tests attempting to write to non-gpr registers on new intel processors (cca Kaby Lake Refresh). Reviewers: jankratochvil, davezarzycki Subscribers: lldb-commits Differential Revision: https://reviews.llvm.org/D59991 ------------------------------------------------------------------------ ------------------------------------------------------------------------ r359120 | josepht | 2019-04-24 11:00:12 -0700 (Wed, 24 Apr 2019) | 15 lines [lldb] Use local definition of get_cpuid_count Summary: This is needed for gcc/cstdlib++ 5.4.0, where __get_cpuid_count is not defined in cpuid.h. Reviewers: labath Reviewed By: labath Subscribers: lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D61036 ------------------------------------------------------------------------ llvm-svn: 359945
1 parent 9465a4c commit 87c0dbb

File tree

2 files changed

+88
-51
lines changed

2 files changed

+88
-51
lines changed

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

Lines changed: 86 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,26 @@
1919

2020
#include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
2121
#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
22-
22+
#include <cpuid.h>
2323
#include <linux/elf.h>
2424

25+
// Newer toolchains define __get_cpuid_count in cpuid.h, but some
26+
// older-but-still-supported ones (e.g. gcc 5.4.0) don't, so we
27+
// define it locally here, following the definition in clang/lib/Headers.
28+
static inline int get_cpuid_count(unsigned int __leaf,
29+
unsigned int __subleaf,
30+
unsigned int *__eax, unsigned int *__ebx,
31+
unsigned int *__ecx, unsigned int *__edx)
32+
{
33+
unsigned int __max_leaf = __get_cpuid_max(__leaf & 0x80000000, 0);
34+
35+
if (__max_leaf == 0 || __max_leaf < __leaf)
36+
return 0;
37+
38+
__cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
39+
return 1;
40+
}
41+
2542
using namespace lldb_private;
2643
using namespace lldb_private::process_linux;
2744

@@ -268,12 +285,29 @@ CreateRegisterInfoInterface(const ArchSpec &target_arch) {
268285
}
269286
}
270287

288+
// Return the size of the XSTATE area supported on this cpu. It is necessary to
289+
// allocate the full size of the area even if we do not use/recognise all of it
290+
// because ptrace(PTRACE_SETREGSET, NT_X86_XSTATE) will refuse to write to it if
291+
// we do not pass it a buffer of sufficient size. The size is always at least
292+
// sizeof(FPR) so that the allocated buffer can be safely cast to FPR*.
293+
static std::size_t GetXSTATESize() {
294+
unsigned int eax, ebx, ecx, edx;
295+
// First check whether the XSTATE are is supported at all.
296+
if (!__get_cpuid(1, &eax, &ebx, &ecx, &edx) || !(ecx & bit_XSAVE))
297+
return sizeof(FPR);
298+
299+
// Then fetch the maximum size of the area.
300+
if (!get_cpuid_count(0x0d, 0, &eax, &ebx, &ecx, &edx))
301+
return sizeof(FPR);
302+
return std::max<std::size_t>(ecx, sizeof(FPR));
303+
}
304+
271305
NativeRegisterContextLinux_x86_64::NativeRegisterContextLinux_x86_64(
272306
const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
273307
: NativeRegisterContextLinux(native_thread,
274308
CreateRegisterInfoInterface(target_arch)),
275-
m_xstate_type(XStateType::Invalid), m_fpr(), m_iovec(), m_ymm_set(),
276-
m_mpx_set(), m_reg_info(), m_gpr_x86_64() {
309+
m_xstate_type(XStateType::Invalid), m_ymm_set(), m_mpx_set(),
310+
m_reg_info(), m_gpr_x86_64() {
277311
// Set up data about ranges of valid registers.
278312
switch (target_arch.GetMachine()) {
279313
case llvm::Triple::x86:
@@ -329,14 +363,13 @@ NativeRegisterContextLinux_x86_64::NativeRegisterContextLinux_x86_64(
329363
break;
330364
}
331365

332-
// Initialize m_iovec to point to the buffer and buffer size using the
333-
// conventions of Berkeley style UIO structures, as required by PTRACE
334-
// extensions.
335-
m_iovec.iov_base = &m_fpr;
336-
m_iovec.iov_len = sizeof(m_fpr);
366+
std::size_t xstate_size = GetXSTATESize();
367+
m_xstate.reset(static_cast<FPR *>(std::malloc(xstate_size)));
368+
m_iovec.iov_base = m_xstate.get();
369+
m_iovec.iov_len = xstate_size;
337370

338371
// Clear out the FPR state.
339-
::memset(&m_fpr, 0, sizeof(m_fpr));
372+
::memset(m_xstate.get(), 0, xstate_size);
340373

341374
// Store byte offset of fctrl (i.e. first register of FPR)
342375
const RegisterInfo *reg_info_fctrl = GetRegisterInfoByName("fctrl");
@@ -439,14 +472,17 @@ NativeRegisterContextLinux_x86_64::ReadRegister(const RegisterInfo *reg_info,
439472

440473
if (byte_order != lldb::eByteOrderInvalid) {
441474
if (reg >= m_reg_info.first_st && reg <= m_reg_info.last_st)
442-
reg_value.SetBytes(m_fpr.fxsave.stmm[reg - m_reg_info.first_st].bytes,
443-
reg_info->byte_size, byte_order);
475+
reg_value.SetBytes(
476+
m_xstate->fxsave.stmm[reg - m_reg_info.first_st].bytes,
477+
reg_info->byte_size, byte_order);
444478
if (reg >= m_reg_info.first_mm && reg <= m_reg_info.last_mm)
445-
reg_value.SetBytes(m_fpr.fxsave.stmm[reg - m_reg_info.first_mm].bytes,
446-
reg_info->byte_size, byte_order);
479+
reg_value.SetBytes(
480+
m_xstate->fxsave.stmm[reg - m_reg_info.first_mm].bytes,
481+
reg_info->byte_size, byte_order);
447482
if (reg >= m_reg_info.first_xmm && reg <= m_reg_info.last_xmm)
448-
reg_value.SetBytes(m_fpr.fxsave.xmm[reg - m_reg_info.first_xmm].bytes,
449-
reg_info->byte_size, byte_order);
483+
reg_value.SetBytes(
484+
m_xstate->fxsave.xmm[reg - m_reg_info.first_xmm].bytes,
485+
reg_info->byte_size, byte_order);
450486
if (reg >= m_reg_info.first_ymm && reg <= m_reg_info.last_ymm) {
451487
// Concatenate ymm using the register halves in xmm.bytes and
452488
// ymmh.bytes
@@ -488,7 +524,7 @@ NativeRegisterContextLinux_x86_64::ReadRegister(const RegisterInfo *reg_info,
488524
return error;
489525
}
490526

491-
// Get pointer to m_fpr.fxsave variable and set the data from it.
527+
// Get pointer to m_xstate->fxsave variable and set the data from it.
492528

493529
// Byte offsets of all registers are calculated wrt 'UserArea' structure.
494530
// However, ReadFPR() reads fpu registers {using ptrace(PTRACE_GETFPREGS,..)}
@@ -499,9 +535,9 @@ NativeRegisterContextLinux_x86_64::ReadRegister(const RegisterInfo *reg_info,
499535
// Since, FPR structure is also one of the member of UserArea structure.
500536
// byte_offset(fpu wrt FPR) = byte_offset(fpu wrt UserArea) -
501537
// byte_offset(fctrl wrt UserArea)
502-
assert((reg_info->byte_offset - m_fctrl_offset_in_userarea) < sizeof(m_fpr));
503-
uint8_t *src =
504-
(uint8_t *)&m_fpr + reg_info->byte_offset - m_fctrl_offset_in_userarea;
538+
assert((reg_info->byte_offset - m_fctrl_offset_in_userarea) < sizeof(FPR));
539+
uint8_t *src = (uint8_t *)m_xstate.get() + reg_info->byte_offset -
540+
m_fctrl_offset_in_userarea;
505541
switch (reg_info->byte_size) {
506542
case 1:
507543
reg_value.SetUInt8(*(uint8_t *)src);
@@ -527,7 +563,7 @@ NativeRegisterContextLinux_x86_64::ReadRegister(const RegisterInfo *reg_info,
527563

528564
void NativeRegisterContextLinux_x86_64::UpdateXSTATEforWrite(
529565
uint32_t reg_index) {
530-
XSAVE_HDR::XFeature &xstate_bv = m_fpr.xsave.header.xstate_bv;
566+
XSAVE_HDR::XFeature &xstate_bv = m_xstate->xsave.header.xstate_bv;
531567
if (IsFPR(reg_index)) {
532568
// IsFPR considers both %st and %xmm registers as floating point, but these
533569
// map to two features. Set both flags, just in case.
@@ -559,15 +595,15 @@ Status NativeRegisterContextLinux_x86_64::WriteRegister(
559595
if (IsFPR(reg_index) || IsAVX(reg_index) || IsMPX(reg_index)) {
560596
if (reg_info->encoding == lldb::eEncodingVector) {
561597
if (reg_index >= m_reg_info.first_st && reg_index <= m_reg_info.last_st)
562-
::memcpy(m_fpr.fxsave.stmm[reg_index - m_reg_info.first_st].bytes,
598+
::memcpy(m_xstate->fxsave.stmm[reg_index - m_reg_info.first_st].bytes,
563599
reg_value.GetBytes(), reg_value.GetByteSize());
564600

565601
if (reg_index >= m_reg_info.first_mm && reg_index <= m_reg_info.last_mm)
566-
::memcpy(m_fpr.fxsave.stmm[reg_index - m_reg_info.first_mm].bytes,
602+
::memcpy(m_xstate->fxsave.stmm[reg_index - m_reg_info.first_mm].bytes,
567603
reg_value.GetBytes(), reg_value.GetByteSize());
568604

569605
if (reg_index >= m_reg_info.first_xmm && reg_index <= m_reg_info.last_xmm)
570-
::memcpy(m_fpr.fxsave.xmm[reg_index - m_reg_info.first_xmm].bytes,
606+
::memcpy(m_xstate->fxsave.xmm[reg_index - m_reg_info.first_xmm].bytes,
571607
reg_value.GetBytes(), reg_value.GetByteSize());
572608

573609
if (reg_index >= m_reg_info.first_ymm &&
@@ -596,7 +632,7 @@ Status NativeRegisterContextLinux_x86_64::WriteRegister(
596632
return Status("CopyMPXtoXSTATE() failed");
597633
}
598634
} else {
599-
// Get pointer to m_fpr.fxsave variable and set the data to it.
635+
// Get pointer to m_xstate->fxsave variable and set the data to it.
600636

601637
// Byte offsets of all registers are calculated wrt 'UserArea' structure.
602638
// However, WriteFPR() takes m_fpr (of type FPR structure) and writes
@@ -608,8 +644,8 @@ Status NativeRegisterContextLinux_x86_64::WriteRegister(
608644
// byte_offset(fpu wrt FPR) = byte_offset(fpu wrt UserArea) -
609645
// byte_offset(fctrl wrt UserArea)
610646
assert((reg_info->byte_offset - m_fctrl_offset_in_userarea) <
611-
sizeof(m_fpr));
612-
uint8_t *dst = (uint8_t *)&m_fpr + reg_info->byte_offset -
647+
sizeof(FPR));
648+
uint8_t *dst = (uint8_t *)m_xstate.get() + reg_info->byte_offset -
613649
m_fctrl_offset_in_userarea;
614650
switch (reg_info->byte_size) {
615651
case 1:
@@ -667,7 +703,7 @@ Status NativeRegisterContextLinux_x86_64::ReadAllRegisterValues(
667703
::memcpy(dst, &m_gpr_x86_64, GetRegisterInfoInterface().GetGPRSize());
668704
dst += GetRegisterInfoInterface().GetGPRSize();
669705
if (m_xstate_type == XStateType::FXSAVE)
670-
::memcpy(dst, &m_fpr.fxsave, sizeof(m_fpr.fxsave));
706+
::memcpy(dst, &m_xstate->fxsave, sizeof(m_xstate->fxsave));
671707
else if (m_xstate_type == XStateType::XSAVE) {
672708
lldb::ByteOrder byte_order = GetByteOrder();
673709

@@ -700,7 +736,7 @@ Status NativeRegisterContextLinux_x86_64::ReadAllRegisterValues(
700736
}
701737
}
702738
// Copy the extended register state including the assembled ymm registers.
703-
::memcpy(dst, &m_fpr, sizeof(m_fpr));
739+
::memcpy(dst, m_xstate.get(), sizeof(FPR));
704740
} else {
705741
assert(false && "how do we save the floating point registers?");
706742
error.SetErrorString("unsure how to save the floating point registers");
@@ -758,9 +794,9 @@ Status NativeRegisterContextLinux_x86_64::WriteAllRegisterValues(
758794

759795
src += GetRegisterInfoInterface().GetGPRSize();
760796
if (m_xstate_type == XStateType::FXSAVE)
761-
::memcpy(&m_fpr.fxsave, src, sizeof(m_fpr.fxsave));
797+
::memcpy(&m_xstate->fxsave, src, sizeof(m_xstate->fxsave));
762798
else if (m_xstate_type == XStateType::XSAVE)
763-
::memcpy(&m_fpr.xsave, src, sizeof(m_fpr.xsave));
799+
::memcpy(&m_xstate->xsave, src, sizeof(m_xstate->xsave));
764800

765801
error = WriteFPR();
766802
if (error.Fail())
@@ -814,12 +850,12 @@ bool NativeRegisterContextLinux_x86_64::IsCPUFeatureAvailable(
814850
return true;
815851
case RegSet::avx: // Check if CPU has AVX and if there is kernel support, by
816852
// reading in the XCR0 area of XSAVE.
817-
if ((m_fpr.xsave.i387.xcr0 & mask_XSTATE_AVX) == mask_XSTATE_AVX)
853+
if ((m_xstate->xsave.i387.xcr0 & mask_XSTATE_AVX) == mask_XSTATE_AVX)
818854
return true;
819855
break;
820856
case RegSet::mpx: // Check if CPU has MPX and if there is kernel support, by
821857
// reading in the XCR0 area of XSAVE.
822-
if ((m_fpr.xsave.i387.xcr0 & mask_XSTATE_MPX) == mask_XSTATE_MPX)
858+
if ((m_xstate->xsave.i387.xcr0 & mask_XSTATE_MPX) == mask_XSTATE_MPX)
823859
return true;
824860
break;
825861
}
@@ -856,10 +892,10 @@ Status NativeRegisterContextLinux_x86_64::WriteFPR() {
856892
switch (m_xstate_type) {
857893
case XStateType::FXSAVE:
858894
return WriteRegisterSet(
859-
&m_iovec, sizeof(m_fpr.fxsave),
895+
&m_iovec, sizeof(m_xstate->fxsave),
860896
fxsr_regset(GetRegisterInfoInterface().GetTargetArchitecture()));
861897
case XStateType::XSAVE:
862-
return WriteRegisterSet(&m_iovec, sizeof(m_fpr.xsave), NT_X86_XSTATE);
898+
return WriteRegisterSet(&m_iovec, sizeof(m_xstate->xsave), NT_X86_XSTATE);
863899
default:
864900
return Status("Unrecognized FPR type.");
865901
}
@@ -879,22 +915,22 @@ bool NativeRegisterContextLinux_x86_64::CopyXSTATEtoYMM(
879915

880916
if (byte_order == lldb::eByteOrderLittle) {
881917
::memcpy(m_ymm_set.ymm[reg_index - m_reg_info.first_ymm].bytes,
882-
m_fpr.fxsave.xmm[reg_index - m_reg_info.first_ymm].bytes,
918+
m_xstate->fxsave.xmm[reg_index - m_reg_info.first_ymm].bytes,
883919
sizeof(XMMReg));
884920
::memcpy(m_ymm_set.ymm[reg_index - m_reg_info.first_ymm].bytes +
885921
sizeof(XMMReg),
886-
m_fpr.xsave.ymmh[reg_index - m_reg_info.first_ymm].bytes,
922+
m_xstate->xsave.ymmh[reg_index - m_reg_info.first_ymm].bytes,
887923
sizeof(YMMHReg));
888924
return true;
889925
}
890926

891927
if (byte_order == lldb::eByteOrderBig) {
892928
::memcpy(m_ymm_set.ymm[reg_index - m_reg_info.first_ymm].bytes +
893929
sizeof(XMMReg),
894-
m_fpr.fxsave.xmm[reg_index - m_reg_info.first_ymm].bytes,
930+
m_xstate->fxsave.xmm[reg_index - m_reg_info.first_ymm].bytes,
895931
sizeof(XMMReg));
896932
::memcpy(m_ymm_set.ymm[reg_index - m_reg_info.first_ymm].bytes,
897-
m_fpr.xsave.ymmh[reg_index - m_reg_info.first_ymm].bytes,
933+
m_xstate->xsave.ymmh[reg_index - m_reg_info.first_ymm].bytes,
898934
sizeof(YMMHReg));
899935
return true;
900936
}
@@ -907,19 +943,19 @@ bool NativeRegisterContextLinux_x86_64::CopyYMMtoXSTATE(
907943
return false;
908944

909945
if (byte_order == lldb::eByteOrderLittle) {
910-
::memcpy(m_fpr.fxsave.xmm[reg - m_reg_info.first_ymm].bytes,
946+
::memcpy(m_xstate->fxsave.xmm[reg - m_reg_info.first_ymm].bytes,
911947
m_ymm_set.ymm[reg - m_reg_info.first_ymm].bytes, sizeof(XMMReg));
912-
::memcpy(m_fpr.xsave.ymmh[reg - m_reg_info.first_ymm].bytes,
948+
::memcpy(m_xstate->xsave.ymmh[reg - m_reg_info.first_ymm].bytes,
913949
m_ymm_set.ymm[reg - m_reg_info.first_ymm].bytes + sizeof(XMMReg),
914950
sizeof(YMMHReg));
915951
return true;
916952
}
917953

918954
if (byte_order == lldb::eByteOrderBig) {
919-
::memcpy(m_fpr.fxsave.xmm[reg - m_reg_info.first_ymm].bytes,
955+
::memcpy(m_xstate->fxsave.xmm[reg - m_reg_info.first_ymm].bytes,
920956
m_ymm_set.ymm[reg - m_reg_info.first_ymm].bytes + sizeof(XMMReg),
921957
sizeof(XMMReg));
922-
::memcpy(m_fpr.xsave.ymmh[reg - m_reg_info.first_ymm].bytes,
958+
::memcpy(m_xstate->xsave.ymmh[reg - m_reg_info.first_ymm].bytes,
923959
m_ymm_set.ymm[reg - m_reg_info.first_ymm].bytes, sizeof(YMMHReg));
924960
return true;
925961
}
@@ -929,7 +965,7 @@ bool NativeRegisterContextLinux_x86_64::CopyYMMtoXSTATE(
929965
void *NativeRegisterContextLinux_x86_64::GetFPRBuffer() {
930966
switch (m_xstate_type) {
931967
case XStateType::FXSAVE:
932-
return &m_fpr.fxsave;
968+
return &m_xstate->fxsave;
933969
case XStateType::XSAVE:
934970
return &m_iovec;
935971
default:
@@ -940,7 +976,7 @@ void *NativeRegisterContextLinux_x86_64::GetFPRBuffer() {
940976
size_t NativeRegisterContextLinux_x86_64::GetFPRSize() {
941977
switch (m_xstate_type) {
942978
case XStateType::FXSAVE:
943-
return sizeof(m_fpr.fxsave);
979+
return sizeof(m_xstate->fxsave);
944980
case XStateType::XSAVE:
945981
return sizeof(m_iovec);
946982
default:
@@ -953,14 +989,14 @@ Status NativeRegisterContextLinux_x86_64::ReadFPR() {
953989

954990
// Probe XSAVE and if it is not supported fall back to FXSAVE.
955991
if (m_xstate_type != XStateType::FXSAVE) {
956-
error = ReadRegisterSet(&m_iovec, sizeof(m_fpr.xsave), NT_X86_XSTATE);
992+
error = ReadRegisterSet(&m_iovec, sizeof(m_xstate->xsave), NT_X86_XSTATE);
957993
if (!error.Fail()) {
958994
m_xstate_type = XStateType::XSAVE;
959995
return error;
960996
}
961997
}
962998
error = ReadRegisterSet(
963-
&m_iovec, sizeof(m_fpr.xsave),
999+
&m_iovec, sizeof(m_xstate->xsave),
9641000
fxsr_regset(GetRegisterInfoInterface().GetTargetArchitecture()));
9651001
if (!error.Fail()) {
9661002
m_xstate_type = XStateType::FXSAVE;
@@ -982,11 +1018,11 @@ bool NativeRegisterContextLinux_x86_64::CopyXSTATEtoMPX(uint32_t reg) {
9821018

9831019
if (reg >= m_reg_info.first_mpxr && reg <= m_reg_info.last_mpxr) {
9841020
::memcpy(m_mpx_set.mpxr[reg - m_reg_info.first_mpxr].bytes,
985-
m_fpr.xsave.mpxr[reg - m_reg_info.first_mpxr].bytes,
1021+
m_xstate->xsave.mpxr[reg - m_reg_info.first_mpxr].bytes,
9861022
sizeof(MPXReg));
9871023
} else {
9881024
::memcpy(m_mpx_set.mpxc[reg - m_reg_info.first_mpxc].bytes,
989-
m_fpr.xsave.mpxc[reg - m_reg_info.first_mpxc].bytes,
1025+
m_xstate->xsave.mpxc[reg - m_reg_info.first_mpxc].bytes,
9901026
sizeof(MPXCsr));
9911027
}
9921028
return true;
@@ -997,10 +1033,10 @@ bool NativeRegisterContextLinux_x86_64::CopyMPXtoXSTATE(uint32_t reg) {
9971033
return false;
9981034

9991035
if (reg >= m_reg_info.first_mpxr && reg <= m_reg_info.last_mpxr) {
1000-
::memcpy(m_fpr.xsave.mpxr[reg - m_reg_info.first_mpxr].bytes,
1036+
::memcpy(m_xstate->xsave.mpxr[reg - m_reg_info.first_mpxr].bytes,
10011037
m_mpx_set.mpxr[reg - m_reg_info.first_mpxr].bytes, sizeof(MPXReg));
10021038
} else {
1003-
::memcpy(m_fpr.xsave.mpxc[reg - m_reg_info.first_mpxc].bytes,
1039+
::memcpy(m_xstate->xsave.mpxc[reg - m_reg_info.first_mpxc].bytes,
10041040
m_mpx_set.mpxc[reg - m_reg_info.first_mpxc].bytes, sizeof(MPXCsr));
10051041
}
10061042
return true;

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ class NativeRegisterContextLinux_x86_64 : public NativeRegisterContextLinux {
109109

110110
// Private member variables.
111111
mutable XStateType m_xstate_type;
112-
FPR m_fpr; // Extended States Area, named FPR for historical reasons.
112+
std::unique_ptr<FPR, llvm::FreeDeleter>
113+
m_xstate; // Extended States Area, named FPR for historical reasons.
113114
struct iovec m_iovec;
114115
YMM m_ymm_set;
115116
MPX m_mpx_set;

0 commit comments

Comments
 (0)